Skip to content

Commit c425686

Browse files
authored
[APMLP-819] Add imageresolver.Config to initialize ImageResolvers (#43308)
### What does this PR do? Adds a new `Config` type in a separate `imageresolver` package. The config should contain all necessary configurations to determine what type of `ImageResolver` implementation should be used. There is no functionality/behavior change from this PR. ### Motivation - **Code clean up** -- Removes the different varieties of RC ImageResolver constructors that made it difficult to follow the code. - **Simpler interface** -- Calling `NewImageResolver()` now only requires the `ImageResolverConfig`, which can be updated later if needed. - **`imageresolver` package** -- Starting this so other ImageResolver specific code can be nested in here in a follow up PR ### Describe how you validated your changes Local E2E validation using `injector-dev` to ensure existing behavior is retained ### Additional Notes Co-authored-by: erika.yasuda <erika.yasuda@datadoghq.com>
1 parent 2c477dc commit c425686

File tree

7 files changed

+172
-67
lines changed

7 files changed

+172
-67
lines changed

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414

1515
"github.com/DataDog/datadog-agent/comp/core/config"
1616
workloadmeta "github.com/DataDog/datadog-agent/comp/core/workloadmeta/def"
17+
"github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/autoinstrumentation/imageresolver"
1718
mutatecommon "github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/common"
1819
configWebhook "github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/config"
1920
"github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/tagsfromlabels"
@@ -28,7 +29,7 @@ func NewAutoInstrumentation(datadogConfig config.Component, wmeta workloadmeta.C
2829
return nil, fmt.Errorf("failed to create auto instrumentation config: %v", err)
2930
}
3031

31-
imageResolver := NewImageResolver(rcClient, datadogConfig)
32+
imageResolver := NewImageResolver(imageresolver.NewConfig(datadogConfig, rcClient))
3233
apm, err := NewTargetMutator(config, wmeta, imageResolver)
3334
if err != nil {
3435
return nil, fmt.Errorf("failed to create auto instrumentation namespace mutator: %v", err)

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

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,12 @@ import (
1717

1818
"github.com/opencontainers/go-digest"
1919

20-
"github.com/DataDog/datadog-agent/comp/core/config"
2120
"github.com/DataDog/datadog-agent/pkg/clusteragent/admission/metrics"
21+
"github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/autoinstrumentation/imageresolver"
2222
"github.com/DataDog/datadog-agent/pkg/remoteconfig/state"
2323
"github.com/DataDog/datadog-agent/pkg/util/log"
2424
)
2525

26-
// RemoteConfigClient defines the interface we need for remote config operations
27-
type RemoteConfigClient interface {
28-
GetConfigs(product string) map[string]state.RawConfig
29-
Subscribe(product string, callback func(map[string]state.RawConfig, func(string, state.ApplyStatus)))
30-
}
31-
3226
// ImageResolver resolves container image references from tag-based to digest-based.
3327
type ImageResolver interface {
3428
// Resolve takes a registry, repository, and tag string (e.g., "gcr.io/datadoghq", "dd-lib-python-init", "v3")
@@ -56,7 +50,7 @@ func (r *noOpImageResolver) Resolve(registry string, repository string, tag stri
5650
// remoteConfigImageResolver resolves image references using remote configuration data.
5751
// It maintains a cache of image mappings received from the remote config service.
5852
type remoteConfigImageResolver struct {
59-
rcClient RemoteConfigClient
53+
rcClient imageresolver.RemoteConfigClient
6054

6155
mu sync.RWMutex
6256
imageMappings map[string]map[string]ImageInfo // repository name -> tag -> resolved image
@@ -67,24 +61,16 @@ type remoteConfigImageResolver struct {
6761
retryDelay time.Duration
6862
}
6963

70-
// newRemoteConfigImageResolver creates a new remoteConfigImageResolver.
71-
// Assumes rcClient is non-nil.
72-
func newRemoteConfigImageResolver(rcClient RemoteConfigClient, datadoghqRegistries map[string]struct{}) ImageResolver {
73-
return newRemoteConfigImageResolverWithRetryConfig(rcClient, 5, 1*time.Second, datadoghqRegistries)
74-
}
75-
76-
// newRemoteConfigImageResolverWithRetryConfig creates a resolver with configurable retry behavior.
77-
// Useful for testing with faster retry settings.
78-
func newRemoteConfigImageResolverWithRetryConfig(rcClient RemoteConfigClient, maxRetries int, retryDelay time.Duration, datadoghqRegistries map[string]struct{}) ImageResolver {
64+
func newRcImageResolver(cfg imageresolver.Config) ImageResolver {
7965
resolver := &remoteConfigImageResolver{
80-
rcClient: rcClient,
66+
rcClient: cfg.RCClient,
8167
imageMappings: make(map[string]map[string]ImageInfo),
82-
maxRetries: maxRetries,
83-
retryDelay: retryDelay,
84-
datadoghqRegistries: datadoghqRegistries,
68+
maxRetries: cfg.MaxInitRetries,
69+
retryDelay: cfg.InitRetryDelay,
70+
datadoghqRegistries: cfg.DDRegistries,
8571
}
8672

87-
rcClient.Subscribe(state.ProductGradualRollout, resolver.processUpdate)
73+
resolver.rcClient.Subscribe(state.ProductGradualRollout, resolver.processUpdate)
8874
log.Debugf("Subscribed to %s", state.ProductGradualRollout)
8975

9076
go func() {
@@ -286,25 +272,16 @@ func newResolvedImage(registry string, repositoryName string, imageInfo ImageInf
286272

287273
// NewImageResolver creates the appropriate ImageResolver based on whether
288274
// a remote config client is available.
289-
func NewImageResolver(rcClient RemoteConfigClient, cfg config.Component) ImageResolver {
290-
291-
if rcClient == nil || reflect.ValueOf(rcClient).IsNil() {
275+
func NewImageResolver(cfg imageresolver.Config) ImageResolver {
276+
if cfg.RCClient == nil || reflect.ValueOf(cfg.RCClient).IsNil() {
292277
log.Debugf("No remote config client available")
293278
return newNoOpImageResolver()
294279
}
295280

296-
datadogRegistriesList := cfg.GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries")
297-
datadogRegistries := newDatadoghqRegistries(datadogRegistriesList)
298-
299-
return newRemoteConfigImageResolverWithDefaultDatadoghqRegistries(rcClient, datadogRegistries)
300-
}
301-
302-
func newRemoteConfigImageResolverWithDefaultDatadoghqRegistries(rcClient RemoteConfigClient, datadoghqRegistries map[string]struct{}) ImageResolver {
303-
resolver := newRemoteConfigImageResolver(rcClient, datadoghqRegistries)
304-
resolver.(*remoteConfigImageResolver).datadoghqRegistries = datadoghqRegistries
305-
return resolver
281+
return newRcImageResolver(cfg)
306282
}
307283

284+
// DEV: Delete this in favor of the imageresolver package one after the refactor is complete
308285
func newDatadoghqRegistries(datadogRegistriesList []string) map[string]struct{} {
309286
datadoghqRegistries := make(map[string]struct{})
310287
for _, registry := range datadogRegistriesList {

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

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/stretchr/testify/require"
2222

2323
"github.com/DataDog/datadog-agent/comp/core/config"
24+
"github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/autoinstrumentation/imageresolver"
2425
"github.com/DataDog/datadog-agent/pkg/remoteconfig/state"
2526
"github.com/DataDog/datadog-agent/pkg/util/log"
2627
)
@@ -107,24 +108,24 @@ func (m *mockRCClient) setBlocking(block bool) {
107108
func TestNewImageResolver(t *testing.T) {
108109
t.Run("with_remote_config_client", func(t *testing.T) {
109110
mockClient := newMockRCClient("image_resolver_multi_repo.json")
110-
mockConfig := config.NewMock(t)
111-
resolver := NewImageResolver(mockClient, mockConfig)
111+
mockConfig := imageresolver.NewConfig(config.NewMock(t), mockClient)
112+
resolver := NewImageResolver(mockConfig)
112113

113114
_, ok := resolver.(*remoteConfigImageResolver)
114115
assert.True(t, ok, "Should return remoteConfigImageResolver when rcClient is not nil")
115116
})
116117

117118
t.Run("without_remote_config_client__typed_nil", func(t *testing.T) {
118-
mockConfig := config.NewMock(t)
119-
resolver := NewImageResolver((*mockRCClient)(nil), mockConfig)
119+
mockConfig := imageresolver.NewConfig(config.NewMock(t), (*mockRCClient)(nil))
120+
resolver := NewImageResolver(mockConfig)
120121

121122
_, ok := resolver.(*noOpImageResolver)
122123
assert.True(t, ok, "Should return noOpImageResolver when rcClient is nil")
123124
})
124125

125126
t.Run("without_remote_config_client__untyped_nil", func(t *testing.T) {
126-
mockConfig := config.NewMock(t)
127-
resolver := NewImageResolver(nil, mockConfig)
127+
mockConfig := imageresolver.NewConfig(config.NewMock(t), nil)
128+
resolver := NewImageResolver(mockConfig)
128129

129130
_, ok := resolver.(*noOpImageResolver)
130131
assert.True(t, ok, "Should return noOpImageResolver when rcClient is nil")
@@ -228,8 +229,7 @@ func TestImageResolverEmptyConfig(t *testing.T) {
228229

229230
func TestRemoteConfigImageResolver_Resolve(t *testing.T) {
230231
mockRCClient := newMockRCClient("image_resolver_multi_repo.json")
231-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
232-
resolver := newRemoteConfigImageResolver(mockRCClient, datadoghqRegistries)
232+
resolver := newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), mockRCClient))
233233

234234
testCases := []struct {
235235
name string
@@ -414,8 +414,7 @@ func TestRemoteConfigImageResolver_InvalidDigestValidation(t *testing.T) {
414414
}
415415

416416
func TestRemoteConfigImageResolver_ConcurrentAccess(t *testing.T) {
417-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
418-
resolver := newRemoteConfigImageResolver(newMockRCClient("image_resolver_multi_repo.json"), datadoghqRegistries).(*remoteConfigImageResolver)
417+
resolver := newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), newMockRCClient("image_resolver_multi_repo.json")))
419418

420419
t.Run("concurrent_read_write", func(_ *testing.T) {
421420
var wg sync.WaitGroup
@@ -437,7 +436,7 @@ func TestRemoteConfigImageResolver_ConcurrentAccess(t *testing.T) {
437436
go func() {
438437
defer wg.Done()
439438
for j := 0; j < 10; j++ {
440-
resolver.processUpdate(map[string]state.RawConfig{}, func(string, state.ApplyStatus) {})
439+
resolver.(*remoteConfigImageResolver).processUpdate(map[string]state.RawConfig{}, func(string, state.ApplyStatus) {})
441440
time.Sleep(10 * time.Millisecond)
442441
}
443442
}()
@@ -495,9 +494,8 @@ func TestAsyncInitialization(t *testing.T) {
495494
t.Run("noop_during_initialization", func(t *testing.T) {
496495
mockClient := newMockRCClient("image_resolver_multi_repo.json")
497496
mockClient.setBlocking(true) // Block initialization
498-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
499497

500-
resolver := newRemoteConfigImageResolverWithRetryConfig(mockClient, 2, 10*time.Millisecond, datadoghqRegistries)
498+
resolver := newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), mockClient))
501499

502500
resolved, ok := resolver.Resolve("gcr.io/datadoghq", "dd-lib-python-init", "latest")
503501
assert.False(t, ok, "Should not complete image resolution during initialization")
@@ -508,8 +506,7 @@ func TestAsyncInitialization(t *testing.T) {
508506
mockClient := newMockRCClient("image_resolver_multi_repo.json")
509507
mockClient.setBlocking(true)
510508

511-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
512-
resolver := newRemoteConfigImageResolverWithRetryConfig(mockClient, 2, 10*time.Millisecond, datadoghqRegistries)
509+
resolver := newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), mockClient))
513510

514511
resolved, ok := resolver.Resolve("gcr.io/datadoghq", "dd-lib-python-init", "latest")
515512
assert.False(t, ok, "Should not complete image resolution during initialization")
@@ -533,8 +530,7 @@ func TestAsyncInitialization(t *testing.T) {
533530
}
534531
close(mockClient.configsReady)
535532

536-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
537-
resolver := newRemoteConfigImageResolverWithRetryConfig(mockClient, 2, 10*time.Millisecond, datadoghqRegistries)
533+
resolver := newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), mockClient))
538534
time.Sleep(50 * time.Millisecond)
539535

540536
resolved, ok := resolver.Resolve("gcr.io/datadoghq", "dd-lib-python-init", "latest")
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
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 kubeapiserver
7+
8+
// Package imageresolver provides configuration and utilities for resolving
9+
// container image references from mutable tags to digests.
10+
package imageresolver
11+
12+
import (
13+
"time"
14+
15+
"github.com/DataDog/datadog-agent/comp/core/config"
16+
"github.com/DataDog/datadog-agent/pkg/remoteconfig/state"
17+
)
18+
19+
// RemoteConfigClient defines the interface we need for remote config operations
20+
type RemoteConfigClient interface {
21+
GetConfigs(product string) map[string]state.RawConfig
22+
Subscribe(product string, callback func(map[string]state.RawConfig, func(string, state.ApplyStatus)))
23+
}
24+
25+
// Config contains information needed to create an ImageResolver
26+
type Config struct {
27+
Site string
28+
DDRegistries map[string]struct{}
29+
RCClient RemoteConfigClient
30+
MaxInitRetries int
31+
InitRetryDelay time.Duration
32+
}
33+
34+
// NewConfig creates a new Config
35+
func NewConfig(cfg config.Component, rcClient RemoteConfigClient) Config {
36+
return Config{
37+
Site: cfg.GetString("site"),
38+
DDRegistries: newDatadoghqRegistries(cfg.GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries")),
39+
RCClient: rcClient,
40+
MaxInitRetries: 5,
41+
InitRetryDelay: 1 * time.Second,
42+
}
43+
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
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 kubeapiserver
7+
8+
package imageresolver
9+
10+
import (
11+
"testing"
12+
"time"
13+
14+
"github.com/stretchr/testify/require"
15+
16+
"github.com/DataDog/datadog-agent/comp/core/config"
17+
)
18+
19+
func TestNewConfig(t *testing.T) {
20+
tests := []struct {
21+
name string
22+
configFactory func(*testing.T) config.Component
23+
expectedState Config
24+
}{
25+
{
26+
name: "default_config",
27+
configFactory: func(t *testing.T) config.Component {
28+
mockConfig := config.NewMock(t)
29+
mockConfig.SetWithoutSource("site", "datadoghq.com")
30+
return mockConfig
31+
},
32+
expectedState: Config{
33+
Site: "datadoghq.com",
34+
DDRegistries: map[string]struct{}{"gcr.io/datadoghq": {}, "docker.io/datadog": {}, "public.ecr.aws/datadog": {}},
35+
RCClient: nil,
36+
MaxInitRetries: 5,
37+
InitRetryDelay: 1 * time.Second,
38+
},
39+
},
40+
{
41+
name: "custom_dd_registries",
42+
configFactory: func(t *testing.T) config.Component {
43+
mockConfig := config.NewMock(t)
44+
mockConfig.SetWithoutSource("site", "datadoghq.com")
45+
mockConfig.SetWithoutSource("admission_controller.auto_instrumentation.default_dd_registries", []string{"helloworld.io/datadog"})
46+
return mockConfig
47+
},
48+
expectedState: Config{
49+
Site: "datadoghq.com",
50+
DDRegistries: map[string]struct{}{"helloworld.io/datadog": {}},
51+
RCClient: nil,
52+
MaxInitRetries: 5,
53+
InitRetryDelay: 1 * time.Second,
54+
},
55+
},
56+
{
57+
name: "configured_site",
58+
configFactory: func(t *testing.T) config.Component {
59+
mockConfig := config.NewMock(t)
60+
mockConfig.SetWithoutSource("site", "datad0g.com")
61+
return mockConfig
62+
},
63+
expectedState: Config{
64+
Site: "datad0g.com",
65+
DDRegistries: map[string]struct{}{"gcr.io/datadoghq": {}, "docker.io/datadog": {}, "public.ecr.aws/datadog": {}},
66+
RCClient: nil,
67+
MaxInitRetries: 5,
68+
InitRetryDelay: 1 * time.Second,
69+
},
70+
},
71+
}
72+
73+
for _, tt := range tests {
74+
t.Run(tt.name, func(t *testing.T) {
75+
mockConfig := tt.configFactory(t)
76+
result := NewConfig(mockConfig, nil)
77+
78+
require.Equal(t, tt.expectedState, result)
79+
})
80+
}
81+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
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 kubeapiserver
7+
8+
// Package imageresolver provides configuration and utilities for resolving
9+
// container image references from mutable tags to digests.
10+
package imageresolver
11+
12+
func newDatadoghqRegistries(datadogRegistriesList []string) map[string]struct{} {
13+
datadoghqRegistries := make(map[string]struct{})
14+
for _, registry := range datadogRegistriesList {
15+
datadoghqRegistries[registry] = struct{}{}
16+
}
17+
return datadoghqRegistries
18+
}

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

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
corev1 "k8s.io/api/core/v1"
1717

1818
"github.com/DataDog/datadog-agent/comp/core/config"
19+
"github.com/DataDog/datadog-agent/pkg/clusteragent/admission/mutate/autoinstrumentation/imageresolver"
1920
"github.com/DataDog/datadog-agent/pkg/util/pointer"
2021
)
2122

@@ -97,13 +98,7 @@ func TestInjectorWithRemoteConfigImageResolver(t *testing.T) {
9798
var resolver ImageResolver
9899
if tc.hasRemoteData {
99100
mockClient := newMockRCClient("image_resolver_multi_repo.json")
100-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
101-
resolver = newRemoteConfigImageResolverWithRetryConfig(
102-
mockClient,
103-
2,
104-
1*time.Millisecond,
105-
datadoghqRegistries,
106-
)
101+
resolver = newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), mockClient))
107102
} else {
108103
resolver = newNoOpImageResolver()
109104
}
@@ -119,13 +114,7 @@ func TestInjectorWithRemoteConfigImageResolver(t *testing.T) {
119114

120115
func TestInjectorWithRemoteConfigImageResolverAfterInit(t *testing.T) {
121116
mockClient := newMockRCClient("image_resolver_multi_repo.json")
122-
datadoghqRegistries := newDatadoghqRegistries(config.NewMock(t).GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries"))
123-
resolver := newRemoteConfigImageResolverWithRetryConfig(
124-
mockClient,
125-
2,
126-
1*time.Millisecond,
127-
datadoghqRegistries,
128-
)
117+
resolver := newRcImageResolver(imageresolver.NewConfig(config.NewMock(t), mockClient))
129118

130119
assert.Eventually(t, func() bool {
131120
_, ok := resolver.Resolve("gcr.io/datadoghq", "apm-inject", "0")

0 commit comments

Comments
 (0)