Skip to content

Commit 7be7893

Browse files
[Backport 7.83.x] Run system-probe config fixups after BuildSchema (#54752)
Backport d019929 from #54738. ___ ### What does this PR do? Renames `fixupInitSystemProbe` to `postProcessSystemProbe` and moves the call from `fixupInitConfig` into `LoadDatadog`, right after `useHostEtc`. ### Motivation `postProcessSystemProbe` reads and writes system-probe config keys (`apt_config_dir`, `yum_repos_dir`, `zypper_repos_dir`) to rewrite them under `HOST_ETC`. It was being called too early on two counts: - Before `BuildSchema()` marks the config ready, so the readiness guard in the getters kicks in and `GetSource`/`GetString` silently no-op. Agents logged `attempt to read key before config is constructed: system_probe_config.apt_config_dir` (and the yum/zypper equivalents) on every start, and the rewrite never applied since `GetSource` returns `SourceUnknown` instead of `SourceDefault` while the config isn't ready. - Before `useHostEtc` runs, so even once that readiness issue is fixed, auto-detected `HOST_ETC` (as opposed to one set in the env ahead of time) still wouldn't be picked up. This was introduced in #53983, which changed the function from a no-op into a real implementation without accounting for either ordering constraint. ### Describe how you validated your changes Added a regression test (`TestPostProcessSystemProbeRunsAfterConfigIsReady`) that runs the real `InitConfigObjects()` + `LoadDatadog()` flow with `HOST_ETC` set and asserts the three keys get rewritten. Confirmed it fails against the old code (reproducing the exact error messages seen in the field) and passes with the fix. Also ran the full `pkg/config/setup` test suite and linter, no regressions. ### Additional Notes Serverless builds don't call this at all, since they have no system-probe config. Co-authored-by: sabrina.lu <sabrina.lu@datadoghq.com>
1 parent 4c39d2f commit 7be7893

5 files changed

Lines changed: 58 additions & 5 deletions

File tree

pkg/config/setup/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ go_library(
6161
dd_agent_go_test(
6262
name = "setup_test",
6363
srcs = [
64+
"config_init_test.go",
6465
"config_secret_test.go",
6566
"config_test.go",
6667
"privateactionrunner_test.go",

pkg/config/setup/config.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,8 @@ func LoadDatadog(config pkgconfigmodel.Config, secretResolver secrets.Component,
523523

524524
useHostEtc(config)
525525

526+
postProcessSystemProbe(SystemProbe())
527+
526528
err = checkConflictingOptions(config)
527529
if err != nil {
528530
return err

pkg/config/setup/config_init.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,4 @@ func fixupInitConfig() {
1919
ddcfg := Datadog()
2020
fixupInitCommonConfigComponents(ddcfg)
2121
fixupInitFullAgentOnlyComponents(ddcfg)
22-
23-
sysprobe := SystemProbe()
24-
fixupInitSystemProbe(sysprobe)
2522
}
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
// Unless explicitly stated otherwise all files in this repository are licensed
2+
// under the Apache License Version 2.0.
3+
// This product includes software developed at Datadog (https://www.datadoghq.com/).
4+
// Copyright 2016-present Datadog, Inc.
5+
6+
//go:build test && !serverless
7+
8+
package setup
9+
10+
import (
11+
"os"
12+
"path/filepath"
13+
"strings"
14+
"testing"
15+
16+
"github.com/stretchr/testify/require"
17+
18+
delegatedauthmock "github.com/DataDog/datadog-agent/comp/core/delegatedauth/mock"
19+
secretsmock "github.com/DataDog/datadog-agent/comp/core/secrets/mock"
20+
pkgconfigmodel "github.com/DataDog/datadog-agent/pkg/config/model"
21+
)
22+
23+
// Regression test: the HOST_ETC repo dir rewrite used to run before the config was ready
24+
// (and before HOST_ETC auto-detection), so it silently never applied.
25+
func TestPostProcessSystemProbeRunsAfterConfigIsReady(t *testing.T) {
26+
origDatadog := Datadog().(pkgconfigmodel.BuildableConfig)
27+
origSystemProbe := SystemProbe().(pkgconfigmodel.BuildableConfig)
28+
t.Cleanup(func() {
29+
SetDatadog(origDatadog) // nolint: forbidigo // restoring the singleton after the test
30+
SetSystemProbe(origSystemProbe) // nolint: forbidigo // restoring the singleton after the test
31+
})
32+
33+
InitConfigObjects()
34+
35+
t.Setenv("HOST_ETC", "/host/etc")
36+
37+
configPath := filepath.Join(t.TempDir(), "empty_conf.yaml")
38+
require.NoError(t, os.WriteFile(configPath, nil, 0o600))
39+
Datadog().(pkgconfigmodel.BuildableConfig).SetConfigFile(configPath)
40+
41+
err := LoadDatadog(Datadog(), secretsmock.New(t), delegatedauthmock.New(t), nil)
42+
require.NoError(t, err)
43+
44+
for _, name := range []string{
45+
"system_probe_config.apt_config_dir",
46+
"system_probe_config.yum_repos_dir",
47+
"system_probe_config.zypper_repos_dir",
48+
} {
49+
val := SystemProbe().GetString(name)
50+
require.True(t, strings.HasPrefix(val, "/host/etc"), "expected %s to be rooted under HOST_ETC, got %q", name, val)
51+
}
52+
}

pkg/config/setup/fixup_init.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,9 @@ func fixupInitFullAgentOnlyComponents(_ pkgconfigmodel.Config) {
117117
pkgconfigmodel.AddOverrideFunc(sanitizeExternalMetricsProviderChunkSize)
118118
}
119119

120-
// called only for system-probe, after declaring settings
121-
func fixupInitSystemProbe(config pkgconfigmodel.Config) {
120+
// postProcessSystemProbe rewrites system-probe repo dir defaults to live under HOST_ETC.
121+
// Called from LoadDatadog, after the config is ready and HOST_ETC has been determined.
122+
func postProcessSystemProbe(config pkgconfigmodel.Config) {
122123
if value, _ := os.LookupEnv("HOST_ETC"); value != "" {
123124
for _, name := range []string{
124125
"system_probe_config.apt_config_dir",

0 commit comments

Comments
 (0)