fix(sinks): separate Datadog validation from runtime construction

Signed-off-by: kurochan <[email protected]>

Author: kurochan

changelog.d/26145_datadog_endpoint_uri_compatibility.fix.md

@@ -0,0 +1,9 @@
+Datadog sink custom endpoints without a scheme now default to `https://`. Query parameters are
+removed when Vector appends its Datadog API path; endpoint query parameters are not supported for
+configuring Datadog API requests.
+
+When migrating a configuration that relies on endpoint query parameters, remove the query string
+and use the Datadog sink's supported request settings or headers instead. Specify `http://` or
+`https://` explicitly when the endpoint must use a particular scheme.
+
+authors: kurochan

changelog.d/26145_datadog_traces_apm_flusher_lifecycle.fix.md

@@ -0,0 +1,6 @@
+Prevent the `datadog_traces` sink from starting its APM statistics flusher while the sink is being
+built. The flusher now follows the sink lifecycle, avoiding leaked background tasks during
+configuration validation or a rolled-back reload. Shutdown waits only a bounded time for the final
+APM statistics flush when the endpoint is unreachable.
+
+authors: kurochan

changelog.d/26145_macos_tls_native_roots_panic.fix.md

@@ -0,0 +1,5 @@
+Prevent configuration validation from panicking on macOS when a restricted environment has no
+available Keychain. Vector avoids loading platform root certificates when TLS certificate
+verification is disabled, unless a client identity requires native certificate-chain support.
+
+authors: kurochan

lib/vector-core/src/tls/settings.rs

@@ -302,25 +302,17 @@ impl TlsSettings {
                 }
             }
         }
-        if self.authorities.is_empty() {
+        if self.should_load_native_roots(for_server) {
             debug!("Fetching system root certs.");
 
             cfg_if! {
                 if #[cfg(windows)] {
-                    load_windows_certs(context).unwrap();
+                    load_windows_certs(context)?;
                 } else if #[cfg(target_os = "macos")] {
-                    cfg_if! { // Panic in release builds, warn in debug builds.
-                        if #[cfg(debug_assertions)] {
-                            if let Err(error) = load_mac_certs(context) {
-                                warn!("Failed to load macOS certs: {error}");
-                            }
-                        } else {
-                            load_mac_certs(context).unwrap();
-                        }
-                    }
+                    load_mac_certs(context)?;
                 }
             }
-        } else {
+        } else if !self.authorities.is_empty() {
             let mut store = X509StoreBuilder::new().context(NewStoreBuilderSnafu)?;
             for authority in &self.authorities {
                 store
@@ -352,6 +344,11 @@ impl TlsSettings {
         Ok(())
     }
 
+    fn should_load_native_roots(&self, for_server: bool) -> bool {
+        self.authorities.is_empty()
+            && (self.verify_certificate || (!for_server && self.identity.is_some()))
+    }
+
     /// Apply per-connection TLS settings.
     ///
     /// `skip_server_name` must be set when the connection targets a forward proxy rather than the
@@ -839,6 +836,37 @@ mod test {
         assert_eq!(settings.authorities.len(), 0);
     }
 
+    #[test]
+    fn apply_context_skips_system_roots_when_verification_is_disabled() {
+        use openssl::ssl::SslMethod;
+
+        let settings = TlsSettings::from_options(Some(&TlsConfig {
+            verify_certificate: Some(false),
+            ..Default::default()
+        }))
+        .expect("Failed to generate TLS settings");
+        let mut context = SslContextBuilder::new(SslMethod::tls()).unwrap();
+
+        // In particular, this must not access the macOS keychain or Windows certificate store.
+        settings
+            .apply_context(&mut context)
+            .expect("TLS context setup should not load native roots when verification is off");
+    }
+
+    #[test]
+    fn client_identity_loads_system_roots_when_verification_is_disabled() {
+        let settings = TlsSettings::from_options(Some(&TlsConfig {
+            verify_certificate: Some(false),
+            crt_file: Some(TEST_PEM_CLIENT_CRT_PATH.into()),
+            key_file: Some(TEST_PEM_CLIENT_KEY_PATH.into()),
+            ..Default::default()
+        }))
+        .expect("Failed to load client identity");
+
+        // Avoid native certificate-store access in this unit test.
+        assert!(settings.should_load_native_roots(false));
+    }
+
     #[test]
     fn from_options_bad_certificate() {
         let options = TlsConfig {

src/sinks/datadog/events/config.rs

@@ -1,7 +1,7 @@
 use http::Uri;
 use indoc::indoc;
 use tower::ServiceBuilder;
-use vector_lib::{config::proxy::ProxyConfig, configurable::configurable_component, schema};
+use vector_lib::{configurable::configurable_component, schema};
 use vrl::value::Kind;
 
 use super::{
@@ -23,7 +23,6 @@ use crate::{
             http::{HttpStatusRetryLogic, RetryStrategy},
         },
     },
-    tls::MaybeTlsSettings,
 };
 
 /// Configuration for the `datadog_events` sink.
@@ -64,12 +63,6 @@ impl DatadogEventsConfig {
             .into_uri())
     }
 
-    fn build_client(&self, proxy: &ProxyConfig) -> crate::Result<HttpClient> {
-        let tls = MaybeTlsSettings::from_config(self.dd_common.tls.as_ref(), false)?;
-        let client = HttpClient::new(tls, proxy)?;
-        Ok(client)
-    }
-
     fn build_sink(
         &self,
         dd_common: &DatadogCommonConfig,
@@ -80,14 +73,13 @@ impl DatadogEventsConfig {
         let service =
             DatadogEventsService::new(endpoint, dd_common.default_api_key.clone(), client);
 
-        let request_settings = validated.request_settings.clone();
         let retry_logic = HttpStatusRetryLogic::new(
             |req: &DatadogEventsResponse| req.http_status,
             self.retry_strategy.clone(),
         );
 
         let service = ServiceBuilder::new()
-            .settings(request_settings, retry_logic)
+            .settings(validated.request_settings.clone(), retry_logic)
             .service(service);
 
         let sink = DatadogEventsSink { service };
@@ -145,9 +137,9 @@ impl ValidatedSink for DatadogEventsConfig {
         validated: &ValidatedEvents,
         cx: SinkContext,
     ) -> crate::Result<(VectorSink, Healthcheck)> {
-        let client = self.build_client(cx.proxy())?;
-        let global = cx.extra_context.get_or_default::<datadog::Options>();
-        let dd_common = self.dd_common.with_globals(global)?;
+        let dd_common = self.dd_common.with_globals_from(&cx)?;
+        // Events defaults to HTTP; explicit TLS configuration overrides it.
+        let client = self.dd_common.build_client(cx.proxy(), false)?;
         let healthcheck = dd_common.build_healthcheck(client.clone())?;
         let endpoint = Self::events_endpoint(dd_common.endpoint.as_deref(), &dd_common.site)?;
         let sink = self.build_sink(&dd_common, client, validated, endpoint)?;

src/sinks/datadog/events/tests.rs

@@ -10,16 +10,30 @@ use vrl::event_path;
 
 use super::*;
 use crate::{
-    config::SinkConfig,
+    config::{SinkConfig, SinkContext},
     event::EventArray,
-    sinks::util::test::{build_test_server_status, load_sink},
+    sinks::util::test::{build_test_server_status, load_sink, load_sink_with_context},
     test_util::{
         addr::next_addr,
         components::{self, COMPONENT_ERROR_TAGS, HTTP_SINK_TAGS},
         random_lines_with_stream,
     },
 };
 
+#[tokio::test]
+async fn pure_validation_does_not_load_tls_files_but_full_build_does() {
+    let config = indoc! {r#"
+        default_api_key = "local-key"
+        tls.enabled = true
+        tls.ca_file = "/definitely/missing/vector-datadog-ca.pem"
+    "#};
+    let (config, cx) =
+        load_sink_with_context::<DatadogEventsConfig>(config, SinkContext::default()).unwrap();
+
+    assert!(crate::config::ValidatedSink::validate(&config).is_ok());
+    assert!(config.build(cx).await.is_err());
+}
+
 fn random_events_with_stream(
     len: usize,
     count: usize,