Skip to content

Commit 5d839f7

Browse files
authored
fix(apm): gate auto CSI by registry (#54401)
### What does this PR do? Restricts CSI selection in automatic APM library injection mode to cases where the injector and every library image use a registry from `default_dd_registries`. Other registries fall back to init-container injection. ### Motivation The CSI driver does not receive the workload's image pull credentials. Selecting it for custom or private registries can therefore make library downloads fail, while init containers can rely on Kubernetes image pull secrets. ### Describe how you validated your changes Ran the auto-instrumentation unit test suite: `dda inv test --targets=./pkg/clusteragent/admission/mutate/autoinstrumentation/...` All 548 targeted tests passed, including new injector and library registry-selection cases. ### Additional Notes Explicit `csi` injection mode remains unchanged. Supporting private registries through CSI requires a separate authentication mechanism. Co-authored-by: luc.vieillescazes <luc.vieillescazes@datadoghq.com>
1 parent db4aa0f commit 5d839f7

7 files changed

Lines changed: 109 additions & 13 deletions

File tree

pkg/clusteragent/admission/mutate/autoinstrumentation/config.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ type staticConfig struct {
5151
// An empty list allows all registries (default).
5252
registryAllowList []string
5353

54+
// defaultDDRegistries contains the Datadog-owned registries that the automatic
55+
// injection mode can safely use through the CSI driver without extra credentials.
56+
defaultDDRegistries []string
57+
5458
// mutateUnlabelled is used to control if we require workloads to have a label when using Local Lib Injection.
5559
mutateUnlabelled bool
5660

@@ -132,6 +136,7 @@ func NewConfig(datadogConfig config.Component) (*Config, error) {
132136

133137
containerRegistry := mutatecommon.ContainerRegistry(datadogConfig, "admission_controller.auto_instrumentation.container_registry")
134138
registryAllowList := datadogConfig.GetStringSlice("admission_controller.auto_instrumentation.container_registry_allow_list")
139+
defaultDDRegistries := datadogConfig.GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries")
135140
mutateUnlabelled := datadogConfig.GetBool("admission_controller.mutate_unlabelled")
136141

137142
return &Config{
@@ -141,6 +146,7 @@ func NewConfig(datadogConfig config.Component) (*Config, error) {
141146
Instrumentation: instrumentationConfig,
142147
containerRegistry: containerRegistry,
143148
registryAllowList: registryAllowList,
149+
defaultDDRegistries: defaultDDRegistries,
144150
mutateUnlabelled: mutateUnlabelled,
145151
initResources: initResources,
146152
initSecurityContext: initSecurityContext,

pkg/clusteragent/admission/mutate/autoinstrumentation/libraryinjection/auto.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
package libraryinjection
99

1010
import (
11+
"slices"
12+
1113
corev1 "k8s.io/api/core/v1"
1214

1315
"github.com/DataDog/datadog-agent/pkg/util/log"
@@ -17,7 +19,7 @@ import (
1719
// It picks the best concrete provider for a pod based on the runtime
1820
// environment, currently:
1921
// - CSIProvider when the Datadog CSI driver is registered in the cluster
20-
// with APM SSI advertised (state cached by CSIDriverWatcher);
22+
// with APM SSI advertised and all images use supported registries;
2123
// - InitContainerProvider otherwise.
2224
type AutoProvider struct {
2325
realProvider LibraryInjectionProvider
@@ -44,15 +46,28 @@ func NewAutoProvider(cfg LibraryInjectionConfig) *AutoProvider {
4446
// the provider behaves exactly as before this feature existed and always
4547
// returns the init-container provider.
4648
func pickAutoProvider(cfg LibraryInjectionConfig) LibraryInjectionProvider {
47-
if cfg.CSIDriverWatcher == nil {
49+
if cfg.CSIDriverWatcher == nil || !cfg.CSIDriverWatcher.IsAPMEnabled() {
50+
log.Debugf("library injection auto provider: Datadog CSI driver %q is unavailable for APM injection, using InitContainerProvider", csiDriverName)
51+
return NewInitContainerProvider(cfg)
52+
}
53+
if registry, found := firstUnsupportedCSIRegistry(cfg); found {
54+
log.Debugf("library injection auto provider: registry %q requires workload image pull credentials, using InitContainerProvider", registry)
4855
return NewInitContainerProvider(cfg)
4956
}
50-
if cfg.CSIDriverWatcher.IsAPMEnabled() {
51-
log.Debugf("library injection auto provider: Datadog CSI driver %q is registered with APM enabled, using CSIProvider", csiDriverName)
52-
return NewCSIProvider(cfg)
57+
log.Debugf("library injection auto provider: Datadog CSI driver %q is registered with APM enabled, using CSIProvider", csiDriverName)
58+
return NewCSIProvider(cfg)
59+
}
60+
61+
func firstUnsupportedCSIRegistry(cfg LibraryInjectionConfig) (string, bool) {
62+
if !slices.Contains(cfg.CSIAutoRegistries, cfg.Injector.Package.Registry) {
63+
return cfg.Injector.Package.Registry, true
64+
}
65+
for _, library := range cfg.Libraries {
66+
if !slices.Contains(cfg.CSIAutoRegistries, library.Package.Registry) {
67+
return library.Package.Registry, true
68+
}
5369
}
54-
log.Debugf("library injection auto provider: Datadog CSI driver %q is not registered (or APM not advertised), using InitContainerProvider", csiDriverName)
55-
return NewInitContainerProvider(cfg)
70+
return "", false
5671
}
5772

5873
// GetName returns the effective injection mode of the resolved concrete provider,

pkg/clusteragent/admission/mutate/autoinstrumentation/libraryinjection/auto_test.go

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020

2121
const datadogCSIDriverName = "k8s.csi.datadoghq.com"
2222

23+
var defaultCSIAutoRegistries = []string{"gcr.io/datadoghq", "public.ecr.aws/datadog"}
24+
2325
// fakeCSIDriverWatcher is a deterministic CSIDriverWatcher implementation
2426
// used in unit tests so we can drive the AutoProvider decision tree without
2527
// spinning up a workloadmeta subscription.
@@ -63,7 +65,9 @@ func TestAutoProvider_PicksCSIWhenWatcherReportsAPMEnabled(t *testing.T) {
6365
pod := newPod()
6466

6567
provider := libraryinjection.NewAutoProvider(libraryinjection.LibraryInjectionConfig{
66-
CSIDriverWatcher: fakeCSIDriverWatcher{registered: true, apmEnabled: true},
68+
Injector: injectorConfig(),
69+
CSIAutoRegistries: defaultCSIAutoRegistries,
70+
CSIDriverWatcher: fakeCSIDriverWatcher{registered: true, apmEnabled: true},
6771
})
6872

6973
result := provider.InjectInjector(pod, injectorConfig())
@@ -78,6 +82,61 @@ func TestAutoProvider_PicksCSIWhenWatcherReportsAPMEnabled(t *testing.T) {
7882
r.Equal(datadogCSIDriverName, vol.CSI.Driver)
7983
}
8084

85+
func TestAutoProvider_RegistrySelection(t *testing.T) {
86+
tests := []struct {
87+
name string
88+
cfg libraryinjection.LibraryInjectionConfig
89+
wantMode string
90+
}{
91+
{
92+
name: "selects csi when every image uses an allowed registry",
93+
cfg: libraryinjection.LibraryInjectionConfig{
94+
Injector: injectorConfig(),
95+
Libraries: []libraryinjection.LibraryConfig{
96+
{Package: libraryinjection.LibraryImage{Registry: "public.ecr.aws/datadog"}},
97+
},
98+
CSIAutoRegistries: []string{"gcr.io/datadoghq", "public.ecr.aws/datadog"},
99+
},
100+
wantMode: "csi (auto)",
101+
},
102+
{
103+
name: "falls back when the injector uses another registry",
104+
cfg: libraryinjection.LibraryInjectionConfig{
105+
Injector: libraryinjection.InjectorConfig{
106+
Package: libraryinjection.LibraryImage{Registry: "registry.example.com/datadog"},
107+
},
108+
CSIAutoRegistries: []string{"gcr.io/datadoghq"},
109+
},
110+
wantMode: "init_container (auto)",
111+
},
112+
{
113+
name: "falls back when a library uses another registry",
114+
cfg: libraryinjection.LibraryInjectionConfig{
115+
Injector: injectorConfig(),
116+
Libraries: []libraryinjection.LibraryConfig{
117+
{Package: libraryinjection.LibraryImage{Registry: "registry.example.com/datadog"}},
118+
},
119+
CSIAutoRegistries: []string{"gcr.io/datadoghq"},
120+
},
121+
wantMode: "init_container (auto)",
122+
},
123+
{
124+
name: "falls back when no csi registry is configured",
125+
cfg: libraryinjection.LibraryInjectionConfig{
126+
Injector: injectorConfig(),
127+
},
128+
wantMode: "init_container (auto)",
129+
},
130+
}
131+
132+
for _, tt := range tests {
133+
t.Run(tt.name, func(t *testing.T) {
134+
tt.cfg.CSIDriverWatcher = fakeCSIDriverWatcher{registered: true, apmEnabled: true}
135+
require.Equal(t, tt.wantMode, libraryinjection.NewAutoProvider(tt.cfg).GetName())
136+
})
137+
}
138+
}
139+
81140
func TestAutoProvider_FallsBackToInitContainerWhenWatcherReportsAPMDisabled(t *testing.T) {
82141
// The watcher knows about the CSI driver but APM is not advertised on it
83142
// (annotation missing, or set to anything other than "true"). AutoProvider

pkg/clusteragent/admission/mutate/autoinstrumentation/libraryinjection/mutator_test.go

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,9 @@ func TestGetName(t *testing.T) {
7070
{
7171
name: "AutoProvider resolves to CSI when driver is available",
7272
provider: libraryinjection.NewAutoProvider(libraryinjection.LibraryInjectionConfig{
73-
CSIDriverWatcher: fakeCSIDriverWatcher{registered: true, apmEnabled: true},
73+
Injector: injectorConfig(),
74+
CSIAutoRegistries: defaultCSIAutoRegistries,
75+
CSIDriverWatcher: fakeCSIDriverWatcher{registered: true, apmEnabled: true},
7476
}),
7577
expected: "csi (auto)",
7678
},
@@ -164,10 +166,11 @@ func TestInjectAPMLibraries_Annotations_Auto_CSI(t *testing.T) {
164166
pod := newPod()
165167

166168
err := libraryinjection.InjectAPMLibraries(pod, libraryinjection.LibraryInjectionConfig{
167-
InjectionMode: string(libraryinjection.InjectionModeAuto),
168-
CSIDriverWatcher: fakeCSIDriverWatcher{registered: true, apmEnabled: true},
169-
Injector: injectorConfig(),
170-
Libraries: []libraryinjection.LibraryConfig{javaLib()},
169+
InjectionMode: string(libraryinjection.InjectionModeAuto),
170+
CSIAutoRegistries: defaultCSIAutoRegistries,
171+
CSIDriverWatcher: fakeCSIDriverWatcher{registered: true, apmEnabled: true},
172+
Injector: injectorConfig(),
173+
Libraries: []libraryinjection.LibraryConfig{javaLib()},
171174
})
172175
require.NoError(t, err)
173176

pkg/clusteragent/admission/mutate/autoinstrumentation/libraryinjection/provider.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,11 @@ type LibraryInjectionConfig struct {
116116
// An empty list allows all registries (default).
117117
RegistryAllowList []string
118118

119+
// CSIAutoRegistries contains registries that auto mode can use through CSI.
120+
// Images from other registries fall back to init containers, which use the
121+
// workload's image pull credentials.
122+
CSIAutoRegistries []string
123+
119124
// CSIDriverWatcher caches the Datadog CSI driver state observed via
120125
// workloadmeta. AutoProvider consults it to decide between CSI and
121126
// init-container injection without hitting workloadmeta on every

pkg/clusteragent/admission/mutate/autoinstrumentation/namespace_mutator.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ func (m *mutatorCore) buildLibraryInjectionConfig(pod *corev1.Pod, config extrac
145145
Debug: m.isDebugEnabled(pod),
146146
AutoDetected: autoDetected,
147147
InjectionType: injectionType,
148+
CSIAutoRegistries: m.config.staticConfig.defaultDDRegistries,
148149
CSIDriverWatcher: m.csiDriverWatcher,
149150
Injector: libraryinjection.InjectorConfig{
150151
Package: injectorImage,
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
fixes:
3+
- |
4+
APM: Make the automatic library injection mode use the CSI driver only when
5+
the injector and library images come from configured Datadog registries.
6+
Images from other registries now fall back to init containers so Kubernetes
7+
can use the workload's image pull credentials.

0 commit comments

Comments
 (0)