fix(feature-flags): add safe agentless EVP fallback - #12299
fix(feature-flags): add safe agentless EVP fallback#12299leoromanovsky wants to merge 10 commits into
Conversation
Route direct feature flag intake through standard HTTPS proxy settings and attach the canonical fixed-width API key fingerprint. Environment: Datadog workspace
|
🎯 Code Coverage (details) 🔗 Commit SHA: 489706c | Docs | View more details | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
Keep this PR focused on proxy-aware direct Event Platform intake and leave fingerprinting to an independent change. Environment: Datadog workspace
Keep the direct-intake diff free of fingerprint-only formatting artifacts. Environment: Datadog workspace
Use the original two-argument direct intake factory now that the unrelated fingerprint header plumbing has moved out of this PR. Environment: Datadog workspace
Reject URL authority confusion before adding DD-API-KEY. Environment: Datadog workspace
Read standard proxy environment variables without registering them as Datadog configuration aliases, and use the existing non-regex parser for no-proxy hosts. Environment: Datadog workspace
Disable HTTP and HTTPS redirects for Feature Flags direct EVP intake so DD-API-KEY remains bound to the configured origin. Environment: Datadog workspace
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 489706ce96
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| noProxyHosts = | ||
| configuredNoProxyHosts == null | ||
| ? Collections.emptySet() | ||
| : parseStringIntoSetOfNonEmptyStrings(configuredNoProxyHosts); |
There was a problem hiding this comment.
Preserve single-character no-proxy entries
When DD_PROXY_NO_PROXY, NO_PROXY, or no_proxy contains a one-character entry, this parser silently omits it because parseStringIntoSetOfNonEmptyStrings only adds tokens whose length is greater than one. In particular, NO_PROXY=* produces an empty set, so the explicit wildcard handling in IntakeProxySelector.shouldBypassProxy is unreachable and direct feature-flag traffic still uses HTTPS_PROXY; single-character internal hostnames also regress from the previous getSpacedList behavior. Parse these values with a helper that retains all nonempty tokens.
Useful? React with 👍 / 👎.
| || normalizedHost.equals(normalized) | ||
| || (normalized.startsWith(".") | ||
| && (normalizedHost.equals(normalized.substring(1)) | ||
| || normalizedHost.endsWith(normalized)))) { |
There was a problem hiding this comment.
Match bare NO_PROXY domains against subdomains
When a standard domain entry such as NO_PROXY=datadoghq.com is used with HTTPS_PROXY, this matcher bypasses only the exact host because suffix matching is restricted to entries starting with a dot. Consequently, event-platform-intake.datadoghq.com is still sent through the proxy, even though bare domain entries in no-proxy lists are expected to cover that domain and its subdomains. Apply boundary-aware suffix matching to bare domain entries as well.
Useful? React with 👍 / 👎.
|
|
||
| // DD_PROXY_NO_PROXY is specified as a space-separated list of hosts | ||
| noProxyHosts = tryMakeImmutableSet(configProvider.getSpacedList(PROXY_NO_PROXY)); | ||
| String configuredHttpsProxy = configProvider.getString(PROXY_HTTPS); |
There was a problem hiding this comment.
Redact credentials from proxy configuration telemetry
When DD_PROXY_HTTPS contains supported userinfo such as http://user:password@proxy:8080, reading it through ConfigProvider.getString records the complete URL in ConfigCollector; ConfigSetting does not classify proxy.https as sensitive, so the telemetry configuration payload serializes the proxy username and password verbatim. Read this setting without collecting the raw value or add explicit redaction for the proxy URL before telemetry emission.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
NO_PROXY=* does not bypass the new intake proxy. The parser removes the wildcard before the proxy selector reads it.
🤖 Datadog Autotest · Commit 489706c · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
| noProxyHosts = | ||
| configuredNoProxyHosts == null | ||
| ? Collections.emptySet() | ||
| : parseStringIntoSetOfNonEmptyStrings(configuredNoProxyHosts); |
There was a problem hiding this comment.
Feature Flag requests with an API key can go through a proxy that the user explicitly disables.
Assertion details
- Input: Set an HTTPS proxy and set
DD_PROXY_NO_PROXY=*,NO_PROXY=*, orno_proxy=*. - Expected:
The wildcard must bypass the proxy for all direct intake hosts. - Actual: The parser removes the one-character
*value. The proxy selector then uses the configured HTTPS proxy.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
Motivation
Java already selects a local EVP route from Agent
/infoand, for agentless Feature Flags, falls back to direct intake for both exposures and flag evaluations. The remaining direct-intake transport has four gaps:DD_PROXY_HTTPSandDD_PROXY_NO_PROXY.DD-API-KEY.DD_SITEis not restricted to the expected HTTPS Event Platform intake origin.Changes
Decisions
Validation
The Java system tests are defined and enabled in DataDog/system-tests#7601.
Ran the Java
spring-bootsystem-test stack against the branch artifact in these scenarios:FEATURE_FLAGGING_AND_EXPERIMENTATIONFEATURE_FLAGGING_AND_EXPERIMENTATION_AGENTLESS_DIRECTFEATURE_FLAGGING_AND_EXPERIMENTATION_AGENTLESS_SERVERLESSIn each topology, the tests exercised both
/api/v2/exposuresand/api/v2/flagevaluation, route-specific headers and authentication, aggregate evaluation counts, and zero events on the unused route.Local validation:
./gradlew :communication:test --tests datadog.communication.BackendApiFactoryTest./gradlew :products:feature-flagging:feature-flagging-lib:test --tests com.datadog.featureflag.FeatureFlagBackendApiFactoryTest --tests com.datadog.featureflag.ExposureWriterTests --tests com.datadog.featureflag.FlagEvaluationWriterImplTest./gradlew :communication:spotlessCheck :communication:forbiddenApisMain :products:feature-flagging:feature-flagging-lib:spotlessCheck :products:feature-flagging:feature-flagging-lib:forbiddenApisMain