[SVLS-7945] feat: Support TLS certificate for logs/proxy flusher #979
Conversation
| } | ||
|
|
||
| // Load custom TLS certificate if configured | ||
| if let Some(cert_path) = &config.tls_cert_file { |
There was a problem hiding this comment.
file and path are confusing, can a customer load more than 1 certificate or define a folder?
There was a problem hiding this comment.
nvm, ignore this, a path can be to a file, but again, my question persist, can a customer load more than 1 cert?
| return Err("No certificates found in file".into()); | ||
| } | ||
|
|
||
| // Convert all certificates found in the file |
There was a problem hiding this comment.
Can a file contain multiple certificates? This confuses me
There was a problem hiding this comment.
A pem file can contain multiple certificates:
https://stackoverflow.com/questions/68340665/pem-file-has-two-certificates-what-does-it-mean
And we supported multiple certs for trace and stats flusher: https://github.com/DataDog/datadog-lambda-extension/blob/main/bottlecap/src/traces/trace_flusher.rs#L247-L250
Not sure if there's a real use case, but since it's not hard, I'm supporting it in this PR.
There was a problem hiding this comment.
Hi! Appreciate the fix, our customer reached out in the support ticket that they do have many CA bundled in one .crt so the use case is valid. They also share that the PR has fixed the issue for them so really appreciate it, thanks a lot!
bottlecap/src/http.rs
Outdated
| let mut reqwest_certs = Vec::new(); | ||
| for cert in certs { | ||
| reqwest_certs.push(reqwest::Certificate::from_der(&cert)?); | ||
| } |
There was a problem hiding this comment.
Is there a more idiomatic way of doing this?
There was a problem hiding this comment.
Good point. Changed to map().
duncanista
left a comment
duncanista
left a comment
There was a problem hiding this comment.
Great job on fixing this! 🙇🏽
lym953
changed the title
…er (#984) ## Motivation A customer reported that their Lambda is behind a proxy, and the Rust-based extension encounters an error when sending logs and metrics to Datadog via the proxy. ## Overview Previous PRs in fixed this for traces/stats (#961) and logs/proxy (#979). DataDog/serverless-components#61 in `serverless-components` repo fixed this issue for dogstatsd metrics. This PR upgrades the dependency on `serverless-components` to patch this update. ## Testing #### Steps See #979 #### Result **Before:** Metrics flushing failed: > DD_EXTENSION | DEBUG | Flushing 0 series and 1 distributions DD_EXTENSION | DEBUG | Sending distributions: SketchPayload { ... } DD_EXTENSION | DEBUG | Request to https://api.datadoghq.com/api/beta/sketches took 144ms DD_EXTENSION | ERROR | Error shipping data: None Failed to send request after 3 attempts DD_EXTENSION | ERROR | Failed to flush some metrics due to shipping errors: 0 series and 1 sketches **After:** Metrics flushing succeeded: > DD_EXTENSION | DEBUG | Flushing 0 series and 1 distributions DD_EXTENSION | DEBUG | Sending distributions: SketchPayload { ... } DD_EXTENSION | DEBUG | Request to https://api.datadoghq.com/api/beta/sketches took 619ms DD_EXTENSION | DEBUG | Successfully flushed 0 series and 1 distributions ## Notes Customer report issue: #919
Problem
A customer reported that their Lambda is behind a proxy, and the Rust-based extension encounters an error when sending logs and metrics to Datadog via the proxy.
A previous PR #961 fixed this for traces and stats, but not for other things because the customer and I didn't see any error with them at that time.
This PR
Applies the env var
DD_TLS_CERT_FILEto logs flusher and proxy flusher as well.Example:
DD_TLS_CERT_FILE=/opt/ca-cert.pem, so the when the extension flushes logs or proxied data to Datadog, the HTTP client created can load and use this cert, and connect the proxy properly.Testing
ngx_http_proxy_connect_moduleca-cert.pemDD_TLS_CERT_FILE=/opt/ca-cert.pemDD_PROXY_HTTPS=http://10.0.0.30:3128, where10.0.0.30is the private IP of the proxy EC2 instanceDD_LOG_LEVEL=debugResult
Before:
Log flushing failed:
After:
No such error
Next steps
Do the same thing for dogstatsd metric flusher. Metric flusher is in a separate repo https://github.com/DataDog/serverless-components, so let's create separate PRs for that change.
Notes
Customer report issue: #919