fix(crypto): gate libdd-common TLS features in remaining internal crates + add CI guard#1943
Conversation
65637a0 to
c5d50bf
Compare
…tes + add CI guard Follows up #1872 by applying the same `default-features = false` + explicit `https`/`fips` feature forwarding pattern to the four internal crates that #1872 missed: * libdd-trace-stats — gates its own libdd-capabilities-impl edge and adds a fips feature. * libdd-data-pipeline — gates its target-cfg libdd-capabilities-impl edge and adds a fips feature forwarding to all libdd-* deps. * libdd-dogstatsd-client — adds a fips feature. * libdd-telemetry — adds a fips feature. Without this, downstream consumers that build with `--no-default-features --features fips` still get `ring` in the dep graph via: ring → rustls → hyper-rustls → libdd-common (https feature, activated by default through libdd-trace-stats → libdd-capabilities-impl) This also affects non-FIPS builds: any consumer that already pulls `rustls/aws-lc-rs` directly (e.g. datadog-lambda-extension) ends up with both `ring` and `aws-lc-rs` linked into the binary, bloating release artifacts and contradicting the gating intent of #1816. To prevent regressions, adds `scripts/check_crypto_providers.sh` plus a CI workflow (`.github/workflows/check-crypto-providers.yml`) that asserts no libdd-* crate ends up with both `ring` and `aws-lc-rs` in its runtime dep graph, in any of: default features, --features https (when exposed), or --features fips (when exposed). The check iterates all libdd-* crates so transitive regressions on crates that do not themselves expose https/fips features are also caught.
c5d50bf to
e7be98f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65637a0be3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The crypto provider check previously returned `not pulled` for every non-zero exit from `cargo tree -i`, which would silently mask real resolution failures (e.g. transient registry/git fetches) and let the guard pass without actually validating the dep graph. Now distinguishes: - exit 0, "<pkg> v..." line -> pulled - exit 0, "nothing to print" -> not pulled (in workspace, not in this graph) - exit non-zero, "did not match" -> not pulled (not in workspace) - anything else -> abort with the cargo output Reported by Codex on PR #1943.
…ored When the cargo registry / git index has not been populated yet (which is the typical state on a fresh CI runner), `cargo tree` prefixes its output with `Downloading crates ...` / `Downloaded ...` progress lines. The previous start-of-stdout match for `<pkg> v...` then failed even when the package WAS in the dep graph, and the new defensive branch turned that into a hard error. Match the heading line with a line-anchored regex so the noise above it does not matter.
🎉 All green!❄️ No new flaky tests detected 🎯 Code Coverage (details) 🔗 Commit SHA: 225c04a | Docs | Datadog PR Page | Give us feedback! |
Artifact Size Benchmark Reportaarch64-alpine-linux-musl
aarch64-unknown-linux-gnu
libdatadog-x64-windows
libdatadog-x86-windows
x86_64-alpine-linux-musl
x86_64-unknown-linux-gnu
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1943 +/- ##
==========================================
+ Coverage 71.75% 71.94% +0.18%
==========================================
Files 434 434
Lines 70450 70450
==========================================
+ Hits 50551 50683 +132
+ Misses 19899 19767 -132
🚀 New features to boost your workflow:
|
| pull_request: | ||
| push: | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
If we're running this on PRs and MQ branches, do we need to run it on main too?
There was a problem hiding this comment.
Yep — main is already in the push: branches: list on line 6 itself. Kept it consistent with how test.yml, lint.yml, fuzz.yml, coverage.yml, etc. all trigger: pull_request + push: branches: [main, mq-working-branch-*]. Running on the post-merge main push gives a baseline status on the default branch and catches anything that slips through (rebase races, MQ skips, etc.) — small CI cost vs. the convention break of opting out only here.
Per review feedback on #1943: the datadog-* / sidecar / live-debugger / remote-config / ffi crates also depend on libdd-common transitively, so a regression in their feature gating could re-introduce the ring + aws-lc-rs co-existence the guard is meant to catch. Switch from a `libdd-*/Cargo.toml` glob to enumerating workspace members via `cargo metadata --no-deps`. The auto-discovery still picks up new crates without maintenance, and now covers 52 members (up from 32 libdd-only) including datadog-sidecar, datadog-ipc, datadog-live-debugger, datadog-remote-config, etc.
There was a problem hiding this comment.
The approach looks good to me, I've been thinking of using the crates reporter action but since we need to compile I think is not worth it a the moment.
One thing I'd suggest is to replace using the regexes in favour of using cargo metadata to extract the features. I left a suggestion about how to do it, LMKWYT.
Per review feedback on #1943: read both the manifest paths AND the declared features from `cargo metadata --no-deps` instead of grepping Cargo.toml separately, so the source of truth is one well-typed structure. * `crate_has_feature` now takes a per-package JSON blob and uses `jq -e '.features | has($feat)'` instead of awk over Cargo.toml. * The main loop reads `{manifest, features}` records into an array via `mapfile -t` + `jq -c`, replacing the python3 one-liner that previously projected just the manifest paths. Same coverage (52 workspace crates), still passes locally.
db05e1f
into
main
…/fips db05e1f is the merge of DataDog/libdatadog#1943 which gates the remaining internal crates that #1872 missed. With those gates in place, bottlecap's `fips` feature can now forward to `libdd-trace-stats/fips` which closes the last path that pulled ring through libdd-capabilities-impl. Verified locally: `cargo clippy --no-default-features --features fips` now reports `No ring dependency found. FIPS compliance check passed`.
…API (#1218) ## Summary Bumps the `libdd-*` git revs in bottlecap from `c8121f42` (~v31.x) to `db05e1f8408a76075efb37ecec544d2e74217e57` (current libdatadog `main`), bumps the `dogstatsd` / `datadog-fips` revs to `5b68f50f49c9defbfed4d25bd621e2a86405a972` (current serverless-components `main`, which already sits on the same libdatadog `db05e1f`), and adapts bottlecap to the breaking changes that ship between those revs. ## What changed and why ### Upstream libdatadog changes that motivated this PR - **DataDog/libdatadog#1555** *(feat: capability traits architecture for HTTP)* — replaced the raw `hyper` client API with an `HttpClientTrait` abstraction. `SendData::send` and `send_with_retry` now require `H: HttpClientTrait`, and `stats_utils::send_stats_payload_with_client` was removed in favor of a generic `send_stats_payload<H: HttpClientTrait>(…)` that constructs its own client. - **`ObfuscationConfig` restructured** — flat fields (`http_remove_path_digits`, `obfuscate_memcached`, …) replaced with nested per-engine structs (`http: HttpConfig`, `memcached: MemcachedConfig`, `redis: RedisConfig`, plus `valkey`, `credit_cards`, `sql`, `elasticsearch`, `opensearch`, `mongodb`). - **DataDog/libdatadog#1816 + #1872 + #1943** *(crypto provider gating)* — moved `ring` behind `libdd-common/https` and `aws-lc-rs` behind `libdd-common/fips`, then progressively gated the internal crates (`libdd-trace-utils`, `libdd-trace-obfuscation`, `libdd-capabilities-impl`, `libdd-trace-stats`, `libdd-data-pipeline`, `libdd-dogstatsd-client`, `libdd-telemetry`) so downstream consumers can pick exactly one provider. **#1943 also added a workspace-wide CI guard** in libdatadog that rejects any PR which puts both `ring` and `aws-lc-rs` in the dep graph at the same time. Other commits in the range (`c8121f42..db05e1f`) are sidecar / FFE / tracer-flare / telemetry changes that don't touch surfaces bottlecap consumes. ### Code changes in this PR #### `bottlecap/src/traces/http_client.rs` — wrap the client to implement `HttpClientTrait` The bottlecap HTTP client must keep proxy + custom CA + skip-SSL support (FIPS, `DD_PROXY_HTTPS`, `DD_TLS_CERT_FILE`, `DD_SKIP_SSL_VALIDATION`). The upstream `DefaultHttpClient` from `libdd-capabilities-impl` hardcodes `Connector::default()` and supports none of those — using it would be a regression. So `HttpClient` is now a newtype around `GenericHttpClient<ProxyConnector<Connector>>` that implements `libdd_capabilities::HttpClientTrait`. The trait's `request()` maps `http::Request<Bytes>` → `Body::from_bytes(…)` → hyper request, then collects the response body back to `Bytes`. ~30 lines, reuses libdatadog's encoding/retry/header logic. `HttpClientTrait::new_client()` is required by the trait but doesn't fit our model (we need a configured client, not a default). It now routes through `create_client(None, None, false)` so the failure surface is consistent with the rest of the module — and it's never invoked on production paths (we always go through `create_client(proxy, tls_cert, skip_ssl)`). #### `bottlecap/src/traces/stats_flusher.rs` — inline the stats POST The new `send_stats_payload<H: HttpClientTrait>(data, target, api_key)` calls `H::new_client()` internally — meaning callers can't supply a pre-configured client anymore. That would lose Lambda's `pool_max_idle_per_host(0)` tuning, which exists specifically to avoid stale connections after Lambda freeze/resume cycles. Replaced the removed call with a tiny in-module `send_stats_payload` helper that builds the same POST request (msgpack + gzip + `DD-API-KEY`) and invokes our `HttpClient`'s `request()` method directly. Per Copilot review: each attempt is wrapped in `tokio::time::timeout(target.timeout_ms, …)` so the retry loop stays bounded by config, and the error-body capture is bounded to 512 bytes (lossy UTF-8) with the HTTP status surfaced in the message instead of silently emptying on non-UTF8 responses. #### `bottlecap/src/bin/bottlecap/main.rs` — flatten → nested `ObfuscationConfig` Maps the two HTTP fields we configure (`apm_config_obfuscation_http_remove_paths_with_digits`, `apm_config_obfuscation_http_remove_query_string`) into the new `HttpConfig`. Everything else flows through `..Default::default()`, which preserves the previous behavior (memcached/redis disabled). #### `bottlecap/Cargo.toml` + `bottlecap/Cargo.lock` - Bumps all `libdd-*` revs to `db05e1f` and `dogstatsd` / `datadog-fips` revs to `5b68f50` (serverless-components `main`). - Adds `libdd-capabilities` (source of `HttpClientTrait`) and `http` (now used directly in `stats_flusher`) as direct dependencies. - Sets `default-features = false` on all `libdd-*` deps and forwards `libdd-*/https` from bottlecap's `default` feature and `libdd-*/fips` from bottlecap's `fips` feature — the consumer-side pattern that #1872's description prescribed. #### `bottlecap/LICENSE-3rdparty.csv` Regenerated via `dd-rust-license-tool write` to include the new `libdd-capabilities` / `libdd-capabilities-impl` / `libdd-shared-runtime` / `http` entries. ## FIPS Previously a known issue — the FIPS clippy job failed because `libdd-trace-stats` (and `libdd-data-pipeline`) pulled `libdd-capabilities-impl` with default features = `https = ring`, with no downstream-side workaround possible. **DataDog/libdatadog#1943 fixed this upstream**, and this PR now forwards `libdd-trace-stats/fips` from the `fips` feature. Verified locally: ``` $ cargo clippy --workspace --all-targets --no-default-features --features fips warning: bottlecap@0.1.0: FIPS feature is enabled, checking for forbidden dependencies... warning: bottlecap@0.1.0: No ring dependency found. FIPS compliance check passed warning: bottlecap@0.1.0: No openssl dependency found. FIPS compliance check passed warning: bottlecap@0.1.0: No boringssl dependency found. FIPS compliance check passed warning: bottlecap@0.1.0: All dependency checks passed. ``` ## Companion PRs (already merged) - **DataDog/libdatadog#1943** — gates the internal libdatadog crates and adds the workspace-wide CI guard. - **DataDog/serverless-components#127** — bumps the `dogstatsd` / `datadog-fips` source workspace to libdatadog `db05e1f`. This PR pins to its merge SHA `5b68f50`, so the whole chain (libdatadog → serverless-components → bottlecap) sits on a consistent baseline. ## Test plan - [x] `cargo check --all-targets` (default features) - [x] `cargo clippy --workspace --all-targets --features default` - [x] `cargo clippy --workspace --all-targets --no-default-features --features fips` — FIPS dependency check now passes locally (was the long-standing CI blocker) - [x] `cargo test --no-run` - [x] `cargo fmt --all -- --check` - [x] Production layer build via `ARCHITECTURE=arm64 FIPS=false ./scripts/build_bottlecap_layer.sh` — built successfully, binary 11.17 MiB stripped / layer zip 5.08 MiB (slightly smaller than `origin/main`'s 11.23 MiB / 5.11 MiB) - [ ] End-to-end smoke test in a real Lambda environment
) ## Summary Stacks on top of #1218. Drops the duplicate `aws-lc-rs` crypto provider from non-FIPS builds, leaving a single backend per mode. Cuts ~15% off the deployed layer zip on its own. | | before | after | Δ | |---|---:|---:|---:| | `bottlecap` stripped (arm64) | 11,709,896 B | **10,137,032 B** | **−1,572,864 B (−13.4%)** | | `datadog_extension.zip` (arm64) | 5,324,844 B | **4,528,013 B** | **−796,831 B (−15.0%)** | ## Why this exists Until #1218 landed the libdatadog gating fixes, bottlecap was pulling **both** `ring` and `aws-lc-rs` into every build: - `ring` came in transitively via `libdd-common/https → rustls/ring + hyper-rustls/ring` - `aws-lc-rs` was hardcoded directly via `rustls = { features = ["aws-lc-rs"] }` in `bottlecap/Cargo.toml`, plus explicit `rustls::crypto::aws_lc_rs::default_provider().install_default()` calls in `http_client.rs` and `trace_processor.rs` The libdatadog gating from DataDog/libdatadog#1943 fixed the *transitive* path, but bottlecap's *direct* `aws-lc-rs` usage remained. Result: the `db05e1f` bump in #1218 keeps both providers linked into the binary and ships ~1.5 MiB of unused machine code (`aws-lc-sys` alone is ~543 KiB stripped per `cargo bloat`, and the surrounding rustls / hyper-rustls / boringssl glue makes up the rest). ## Change 1. **`bottlecap/Cargo.toml`** — drop the `aws-lc-rs` feature from the rustls direct dep: ```diff -rustls = { version = "0.23.18", default-features = false, features = ["aws-lc-rs"] } +rustls = { version = "0.23.18", default-features = false } ``` The provider now flows in transitively per mode: - `default`: `libdd-common/https` → `rustls/ring` - `fips`: `bottlecap/fips` → `rustls/fips` (FIPS-validated aws-lc-rs) 2. **`bottlecap/src/traces/http_client.rs`** — make `ensure_crypto_provider_initialized` cfg-conditional on the bottlecap `fips` feature so `rustls::crypto::ring::default_provider` is used in non-FIPS and `rustls::crypto::aws_lc_rs::default_provider` only in FIPS. 3. **`bottlecap/src/traces/trace_processor.rs`** — same cfg split for the test-helper `default_provider().install_default()` call. ## Verification `cargo tree`: ``` $ cargo tree -i ring --no-default-features --features default ring v0.17.14 └── ... (via libdd-common/https path) ✓ pulled $ cargo tree -i aws-lc-rs --no-default-features --features default warning: nothing to print. ✓ absent $ cargo tree -i ring --no-default-features --features fips warning: nothing to print. ✓ absent $ cargo tree -i aws-lc-rs --no-default-features --features fips aws-lc-rs v1.16.2 └── ... ✓ pulled ``` Build script's FIPS dep check still passes: ``` $ cargo clippy --workspace --all-targets --no-default-features --features fips warning: bottlecap@0.1.0: No ring dependency found. FIPS compliance check passed warning: bottlecap@0.1.0: No openssl dependency found. FIPS compliance check passed warning: bottlecap@0.1.0: No boringssl dependency found. FIPS compliance check passed warning: bottlecap@0.1.0: All dependency checks passed. ``` ## Risk This is a real behavior change for non-FIPS deployments — TLS goes from `aws-lc-rs` to `ring` as the default provider. Both are FIPS-incapable in non-FIPS builds anyway (the FIPS variant lives behind `rustls/fips`), so the user-visible difference is limited to TLS internals (cipher suite priority list, perf characteristics, supported algorithms). `ring` is the libdatadog-wide default and is what every other libdd-* consumer in the workspace ends up on after #1943, so this aligns bottlecap with the rest of the ecosystem. Should be smoke-tested in a real Lambda environment before merging — the dev-server path and a representative trace/metric flush should both work end-to-end. ## Test plan - [x] `cargo check --all-targets` (default features) - [x] `cargo clippy --workspace --all-targets --features default` - [x] `cargo clippy --workspace --all-targets --no-default-features --features fips` — FIPS dep check still passes - [x] `cargo tree` — verified single provider per mode - [x] Production layer build via `ARCHITECTURE=arm64 FIPS=false ./scripts/build_bottlecap_layer.sh` — built successfully, sizes above - [ ] End-to-end smoke test in a real Lambda environment (TLS handshake, trace/metric/log flushes) - [ ] FIPS production build smoke test ## Base Stacked on #1218 (libdatadog `db05e1f` bump + HttpClientTrait adoption). Will rebase to `main` once that lands.
# Release proposal for libdd-telemetry and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-capabilities **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-capabilities-v2.0.0` ### Commits - feat(capablities)!: sleep & spawn capabilities (#1873) ## libdd-common **Next version:** `4.1.0` **Semver bump:** `minor` **Tag:** `libdd-common-v4.1.0` ### Commits - refactor(sampling): move the sampling logic from dd-trace-rs [APMSP-2946] (#1927) - feat!: added regex-lite feature (#1939) - fix(libdd-common): crashes caused by `getenv` while retrieving AAS env vars (#1930) ## libdd-capabilities-impl **Next version:** `2.0.0` **Semver bump:** `major` **Tag:** `libdd-capabilities-impl-v2.0.0` ### Commits - feat(capablities)!: sleep & spawn capabilities (#1873) ## libdd-shared-runtime **Next version:** `1.0.0` **Semver bump:** `major` **Tag:** `libdd-shared-runtime-v1.0.0` **Warning:** this is an initial release. Please verify that the version and commits included are correct. ## libdd-telemetry **Next version:** `5.0.0` **Semver bump:** `major` **Tag:** `libdd-telemetry-v5.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^3.0.2 → ^4.0.0 ### Commits - fix(libdd-telemetry): restore previous Cargo.toml version (#1993) - feat(capablities)!: sleep & spawn capabilities (#1873) - fix(telemetry): avoid trigger loop in telemetry worker (#1950) - feat(telemetry)!: include dependencies and integrations in app-extended-heartbeat (#1962) - fix(crypto): gate libdd-common TLS features in remaining internal crates + add CI guard (#1943) - fix(telemetry): schedule ExtendedHeartbeat on worker start (#1910) - fix(telemetry): skip sending empty payloads (#1894) - feat(sidecar): wire telemetry_extended_heartbeat_interval through SessionConfig (#1882) - chore(telemetry): add session id support (#1817) - ci(libdd-shared-runtime): downgrade version so publish workflow succeeds (#1870) - feat(runtime)!: add shared runtime (#1602) - perf(sidecar)!: Batch ack sending & consumption (#1835) - fix(telemetry): wire up DD_TELEMETRY_EXTENDED_HEARTBEAT_INTERVAL to scheduler (#1824) - feat(capabilities)!: trait architecture http (#1555) - chore(telemetry): use weaker mem ordering for SEQ_ID (#1749) [APMSP-2946]: https://datadoghq.atlassian.net/browse/APMSP-2946?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: iunanua <18325288+iunanua@users.noreply.github.com>
# Release proposal for libdd-data-pipeline and its dependencies This PR contains version bumps based on public API changes and commits since last release. ## libdd-dogstatsd-client **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-dogstatsd-client-v3.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^3.0.2 → ^4.1.0 ### Commits - fix(crypto): gate libdd-common TLS features in remaining internal crates + add CI guard (#1943) ## libdd-trace-obfuscation **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-obfuscation-v3.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^3.0.2 → ^4.1.0 - `libdd-trace-utils`: ^3.0.1 → ^4.0.0 ### Commits - feat!: integrate obfuscation to the stats exporter [APMSP-2764] (#1819) - feat!: added regex-lite feature (#1939) - chore: clippy (#1889) - fix(crypto): gate libdd-common TLS features in obfuscation and capabilities-impl (#1872) - feat(obfuscation)!: feature parity on span obfuscation [APMSP-2671] (#1788) - feat(obfuscation/sql): feature parity on sql obfuscation [APMSP-2667] (#1708) ## libdd-trace-stats **Next version:** `3.0.0` **Semver bump:** `major` **Tag:** `libdd-trace-stats-v3.0.0` ###⚠️ major bump forced due to: - `libdd-trace-utils`: ^3.0.1 → ^4.0.0 ### Commits - perf: pre-compute string messagepack encoding (#1948) - feat!: integrate obfuscation to the stats exporter [APMSP-2764] (#1819) - feat(capablities)!: sleep & spawn capabilities (#1873) - feat: use ip quantization when aggregating peer tags for trace stats (#1944) - fix(crypto): gate libdd-common TLS features in remaining internal crates + add CI guard (#1943) - feat(shared_runtime)!: allow worker to be stopped after fork (#1893) - feat(sidecar)!: Add stats computation via SHM (#1821) - feat(stats): propagate service source from span meta to client stats payload (#1803) - fix(stats): align with css spec (#1790) ## libdd-data-pipeline **Next version:** `4.0.0` **Semver bump:** `major` **Tag:** `libdd-data-pipeline-v4.0.0` ###⚠️ major bump forced due to: - `libdd-common`: ^3.0.2 → ^4.1.0 - `libdd-telemetry`: ^4.0.0 → ^5.0.0 - `libdd-trace-utils`: ^3.0.1 → ^4.0.0 ### Commits - fix(libdd-telemetry): restore previous Cargo.toml version (#1993) - fix(data-pipeline): remove default-features from of trace-obfuscation (#1981) - fix(trace_exporter: shared_runtime): unwrap_or being eager is not good (#1983) - perf: pre-compute string messagepack encoding (#1948) - feat!: integrate obfuscation to the stats exporter [APMSP-2764] (#1819) - feat(capablities)!: sleep & spawn capabilities (#1873) - fix(telemetry): avoid trigger loop in telemetry worker (#1950) - feat(telemetry)!: include dependencies and integrations in app-extended-heartbeat (#1962) - perf(trace-serializer): pre-allocate serialization buffer (#1949) - feat!: added regex-lite feature (#1939) - fix(crypto): gate libdd-common TLS features in remaining internal crates + add CI guard (#1943) - feat(telemetry): add session id support to trace export (#1822) - fix(path): missing bench path in data-pipeline (#1907) - feat(data-pipeline): port dd-trace-rs trace buffer implementation (#1826) - feat(info_fetcher): add timeout to info fetcher (#1890) - feat(shared_runtime)!: allow worker to be stopped after fork (#1893) - feat(sidecar)!: Add stats computation via SHM (#1821) - ci(libdd-shared-runtime): downgrade version so publish workflow succeeds (#1870) - feat(runtime)!: add shared runtime (#1602) - ci: compilation of libdd-data-pipeline to wasm32 (#1830) - feat(capabilities)!: trait architecture http (#1555) - feat(otel): add support for OTLP trace export (#1641) - fix(stats): align with css spec (#1790) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: iunanua <18325288+iunanua@users.noreply.github.com>
What does this PR do?
Follows up #1872 by applying the same
default-features = false+ explicithttps/fipsfeature forwarding pattern to the four internal crates that #1872 missed, and adds a CI guard so this class of regression cannot reland silently.Cargo.toml changes
libdd-trace-statslibdd-capabilities-implungated; nofipsfeaturedefault-features = false+https/fipsfeatures forwarding tolibdd-commonandlibdd-capabilities-impllibdd-data-pipelinelibdd-capabilities-implungated; nofipsfeaturedefault-features = false+ newfipsfeature mirroring the existinghttpsforwarding (libdd-common,libdd-capabilities-impl,libdd-telemetry?,libdd-trace-stats,libdd-trace-utils,libdd-dogstatsd-client)libdd-dogstatsd-clientfipsfeaturefips = ["libdd-common/fips"]libdd-telemetryfipsfeaturefips = ["libdd-common/fips"]CI guard
Adds
scripts/check_crypto_providers.shplus.github/workflows/check-crypto-providers.yml. The script asserts that nolibdd-*crate ends up with bothringandaws-lc-rsin its runtime dependency graph, in any of these build modes:cargopicks)--no-default-features --features https(when the crate exposes anhttpsfeature)--no-default-features --features fips(when the crate exposes afipsfeature)It iterates all
libdd-*crates so a regression introduced via a transitive edge — even on a crate that doesn't itself exposehttps/fips— gets flagged. Resolution is per-crate (via--manifest-path) so workspace-level feature unification from other members can't mask a violation, and dev-deps are excluded so test-only graphs don't produce false positives. Locally on this branch:crypto provider check passed for 32 crate(s).Motivation
Without these gates, downstream consumers that build with
--no-default-features --features fipsstill getringin the dep graph through:This is the same shape #1872 fixed for
libdd-trace-obfuscation,libdd-capabilities-impl, andlibdd-trace-utils; these four crates were left out. It also affects non-FIPS builds: any consumer that pullsrustls/aws-lc-rsdirectly (e.g.datadog-lambda-extension) ends up with bothringandaws-lc-rslinked into the binary, bloating release artifacts and contradicting the gating intent of #1816.How to test the change?
Default builds are unchanged (the
default = ["https"]feature on each crate preserves the existingringbehavior).Verify ring is absent from FIPS feature builds of the affected crates:
Run the full guard locally:
Sanity-check the guard catches regressions — temporarily drop
default-features = falsefromlibdd-trace-stats'slibdd-capabilities-impledge and re-run the script. It correctly fails withlibdd-trace-stats (--features fips) pulls both ring and aws-lc-rsandlibdd-data-pipeline (--features fips) pulls both ring and aws-lc-rs.