Revert "fix(data plane): ADP preflight ignores DD_SITE (APMSP-3994)" (#55909)

Automatic revert of commit 4f136f4.
#55886
This PR is created to anticipate the resolution of
`new-e2e-adp-preflight-mode` job failure in `main` branch in case of
erroneous code change.
If this revert is incorrect, do not hesitate to close the PR.

Very crisp breakage at the original change.

https://app.datadoghq.com/ci/ci-cd/explorer?query=ci_level%3Ajob%20%40ci.pipeline.name%3ADataDog%2Fdatadog-agent%20%40git.branch%3Amain%20%40ci.job.name%3A%22new-e2e-adp-preflight-mode%22&agg_m=%40ci.job.name&agg_m_source=base&agg_t=cardinality&analyticsOptions=%5B%22line%22%2C%22dog_classic%22%2Cnull%2Cnull%2C%22value%22%5D&ci_cd_explorer_tab=pipelines&cipipeline_explorer_sort=time%2Cdesc&colorByAttr=meta%5B%27ci.stage.name%27%5D&cols=%40git.branch%2C%40ci.status%2Ctimestamp%2C%40ci.pipeline.name%2C%40ci.stage.name%2C%40ci.job.name%2C%40duration%2C%40ci.pipeline.id%2C%40git.repository.name&currentTab=trace&fromUser=false&graphType=flamegraph&index=cipipeline&mode=sliding&refresh_mode=sliding&sort=time&spanViewType=metadata&step=86400000&tab=overview&viz=stream&start=1783394387204&end=1788578387204&paused=false

Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com>
Co-authored-by: Tony Aiuto <tony.aiuto@datadoghq.com>

Author: datadog-datadog-prod-us1[bot]

comp/dataplane/preflightmode/impl/config.go

@@ -116,22 +116,13 @@ var preflightModeGlobalOverrides = map[string]any{
 // buildPreflightConfig returns the configuration ADP should run with during a preflight mode
 // pre-flight.
 //
-// The Agent configuration as the operator supplied it is used as the base, and overrides are
-// applied on top of it: this ensures that ADP is configured as close as possible to how it would
-// be when running normally, with only the necessary changes to run it in "preflight" mode:
-// don't take over DSD, don't run any other pipelines, don't log to disk, and don't reserve buffer
-// pools sized for traffic that a one-metric pre-flight will never see.
-//
-// Deliberately AllSettingsWithoutDefault and not AllSettings. A normally-supervised ADP is started
-// with `--config /etc/datadog-agent/datadog.yaml`, so it sees the operator's settings and fills in
-// the rest from its own defaults; handing it the Agent's defaults instead would be a different
-// configuration, not a more faithful one. It is also incorrect: AllSettings renders a default
-// indistinguishably from a value the operator asked for, and dd_url's default is a non-empty
-// https://app.datadoghq.com. Since an explicit dd_url beats `site` in ADP just as it does in the
-// Core Agent, a site-only config came out of AllSettings pointing the pre-flight -- metrics and
-// API key both -- at US1 no matter what site the operator had configured.
+// The full Agent configuration as it exists at the time of this call is used as the base,
+// and overrides are applied on top of it: this ensures that ADP is configured as close as possible
+// to how it would be when running normally, with only the necessary changes to run it in "preflight" mode:
+// don't take over DSD, don't run any other pipelines, don't log to disk, and don't reserve buffer pools
+// sized for traffic that a one-metric pre-flight will never see.
 func buildPreflightConfig(cfg pkgconfigmodel.Reader, l listener) map[string]any {
-	out := cfg.AllSettingsWithoutDefault()
+	out := cfg.AllSettings()
 	if out == nil {
 		out = map[string]any{}
 	}
@@ -157,14 +148,12 @@ func buildPreflightConfig(cfg pkgconfigmodel.Reader, l listener) map[string]any
 
 // writePreflightConfig renders the preflight configuration into workDir and returns its path.
 //
-// The file holds every setting the operator supplied -- see buildPreflightConfig for why the
-// Agent's own defaults are deliberately left out -- so every credential the Agent was given in
-// plain text -- api_key, app_key, proxy credentials, additional_endpoints keys -- ends up in it.
-// Not, however, anything from a secret backend: dropping the defaults layer does not drop the
-// secrets layer, so AllSettingsWithoutDefault would still render a resolved secret, but
-// isEligible refuses to run the pre-flight at all when secrets are in use, precisely so that
-// this file cannot be how a secret first reaches the disk. The working directory is removed when
-// the run finishes, and again from stop if the run does not unwind in time.
+// The file holds the Agent's entire resolved configuration, so every credential the Agent was
+// given in plain text -- api_key, app_key, proxy credentials, additional_endpoints keys -- ends
+// up in it. Not, however, anything from a secret backend: AllSettings would merge the secrets
+// layer, but isEligible refuses to run the pre-flight at all when secrets are in use, precisely
+// so that this file cannot be how a secret first reaches the disk. The working directory is
+// removed when the run finishes, and again from stop if the run does not unwind in time.
 //
 // The 0600 below is what restricts the file on Unix. It does nothing on Windows, where the
 // mode is not an access control mechanism at all; there the file is covered by the ACL

comp/dataplane/preflightmode/impl/config_test.go

@@ -73,84 +73,26 @@ func TestBuildPreflightConfigCarriesOperatorSettings(t *testing.T) {
 	requireEq(t, got, "proxy.https", "http://proxy.internal:3128")
 }
 
-// TestBuildPreflightConfigCarriesOnlyOperatorSettings documents the choice to base the preflight
-// config on AllSettingsWithoutDefault: everything the operator configured, and nothing they did
-// not.
+// TestBuildPreflightConfigCarriesTheWholeAgentConfig documents the deliberate choice to base
+// the preflight config on AllSettings, defaults included, rather than only the settings the
+// operator touched.
 //
-// The point of the pre-flight is for ADP to see the configuration it would really run with, and a
-// normally-supervised ADP is started with `--config /etc/datadog-agent/datadog.yaml` -- the
-// operator's settings, with ADP's own defaults underneath. Passing the Agent's defaults instead
-// would be a different configuration, not a more faithful one, and for dd_url it was an actively
-// wrong one (see TestBuildPreflightConfigDropsUnsetDDURL).
-//
-// Passing the operator's settings through wholesale is safe because ADP ignores keys it does not
-// recognise: verified against agent-data-plane 1.4.0 by running it with the full Core Agent
-// config, including Core-Agent-only sections, with no resulting warning or error (see
-// TestBuildPreflightConfigPassesThroughCoreAgentOnlySettings).
-func TestBuildPreflightConfigCarriesOnlyOperatorSettings(t *testing.T) {
-	cfg := configmock.New(t)
-	cfg.Set("forwarder_timeout", 42, pkgconfigmodel.SourceFile)
-
-	got := buildPreflightConfig(cfg, newListener(t.TempDir()))
-
-	// A setting the operator tuned reaches ADP, so it forwards the way the Agent would.
-	requireEq(t, got, "forwarder_timeout", 42)
-
-	// One they never touched does not, so ADP applies its own default rather than the Agent's.
-	_, present := get(t, got, "logs_config")
-	assert.False(t, present, "a section the operator never configured should not reach ADP")
-}
-
-// TestBuildPreflightConfigCarriesEnvSourcedSettings is the case that matters most in a container,
-// where the operator configures the Agent entirely through DD_ variables and datadog.yaml is
-// close to empty.
-//
-// It is load-bearing for the pre-flight specifically: childEnv strips the whole DD_ namespace from
-// the preflight process, so the generated file is the only channel these settings have. A dump
-// that dropped them would leave ADP with no api_key and no site at all.
-func TestBuildPreflightConfigCarriesEnvSourcedSettings(t *testing.T) {
-	t.Setenv("DD_API_KEY", "0123456789abcdef0123456789abcdef")
-	t.Setenv("DD_SITE", "datad0g.com")
-
-	cfg := configmock.New(t)
-	// The Agent resolves DD_PROXY_* into the config at startup rather than leaving it on the env
-	// layer, writing it back as config-post-init; mirror that here so the dump under test is the
-	// one the real preflight sees.
-	cfg.Set("proxy.https", "http://proxy.internal:3128", pkgconfigmodel.SourceConfigPostInit)
-
-	got := buildPreflightConfig(cfg, newListener(t.TempDir()))
-
-	requireEq(t, got, "api_key", "0123456789abcdef0123456789abcdef")
-	requireEq(t, got, "site", "datad0g.com")
-	requireEq(t, got, "proxy.https", "http://proxy.internal:3128")
-}
-
-// TestBuildPreflightConfigDropsUnsetDDURL is the regression this policy exists for.
-//
-// dd_url's default is a non-empty https://app.datadoghq.com, and an explicit dd_url beats `site`
-// in ADP just as it does in the Core Agent. Rendered as a default by AllSettings, it pointed the
-// pre-flight -- metrics and API key both -- at US1 regardless of the configured site.
-func TestBuildPreflightConfigDropsUnsetDDURL(t *testing.T) {
-	cfg := configmock.New(t)
-	cfg.Set("site", "datad0g.com", pkgconfigmodel.SourceFile)
-
-	got := buildPreflightConfig(cfg, newListener(t.TempDir()))
-
-	_, present := get(t, got, "dd_url")
-	assert.False(t, present, "an unset dd_url must not reach ADP as though it were configured")
-	requireEq(t, got, "site", "datad0g.com")
-}
-
-// TestBuildPreflightConfigKeepsConfiguredDDURL is the other half: a dd_url the operator really set
-// is part of the configuration ADP is meant to run against, and must survive.
-func TestBuildPreflightConfigKeepsConfiguredDDURL(t *testing.T) {
+// The point of the pre-flight is for ADP to see the configuration it would really run with,
+// and passing everything through is the closest approximation. It is safe because ADP
+// ignores keys it does not recognise: verified against agent-data-plane 1.4.0 by running it
+// with the full Core Agent config, including Core-Agent-only sections, with no resulting
+// warning or error (see TestBuildPreflightConfigPassesThroughCoreAgentOnlySettings).
+func TestBuildPreflightConfigCarriesTheWholeAgentConfig(t *testing.T) {
 	cfg := configmock.New(t)
-	cfg.Set("site", "datad0g.com", pkgconfigmodel.SourceFile)
-	cfg.Set("dd_url", "https://app.datad0g.com", pkgconfigmodel.SourceFile)
 
 	got := buildPreflightConfig(cfg, newListener(t.TempDir()))
 
-	requireEq(t, got, "dd_url", "https://app.datad0g.com")
+	// Defaults ADP shares with the Agent come through, so it forwards the way the Agent
+	// would.
+	_, present := get(t, got, "forwarder_timeout")
+	assert.True(t, present, "the Agent's forwarder settings should reach ADP")
+	_, present = get(t, got, "site")
+	assert.True(t, present, "the Agent's site should reach ADP")
 }
 
 func TestBuildPreflightConfigOverrides(t *testing.T) {

comp/dataplane/preflightmode/impl/eligible.go

@@ -61,8 +61,8 @@ const (
 // secretsInUse reports whether the Agent's configuration involves secrets at all, along with a human-readable reason
 // when it does.
 //
-// This is a gate on preflight mode because the pre-flight writes the Agent's configuration as the operator supplied it
-// to disk for ADP to read (see writePreflightConfig). Anything the secret resolver pulled into memory would therefore be
+// This is a gate on preflight mode because the pre-flight writes the Agent's entire resolved configuration to disk for
+// ADP to read (see writePreflightConfig). Anything the secret resolver pulled into memory would therefore be
 // materialized in plaintext in a file. The working directory is locked down to the Agent's own account and removed when
 // the run finishes, but a value the operator deliberately kept out of any file on the host should not be written to one
 // for the sake of a pre-flight check that only exists until ADP goes GA. So when secrets are involved we skip the