-
Notifications
You must be signed in to change notification settings - Fork 1.4k
[APMLP-819] Add imageresolver.Config to initialize ImageResolvers
#43308
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
160b3f2
9e11533
88fd768
a060afe
9e53643
74dbd9b
9296434
61db2a2
f4a874e
59ee119
479b0db
be61aec
4026dbb
91fd8e8
b749f15
8bb35b6
8e501a7
d887719
825728a
6f33f6f
28c8c78
d05ae4b
57878bd
d250361
90244eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2016-present Datadog, Inc. | ||
|
|
||
| //go:build kubeapiserver | ||
|
|
||
| // Package imageresolver provides configuration and utilities for resolving | ||
| // container image references from mutable tags to digests. | ||
| package imageresolver | ||
|
|
||
| import ( | ||
| "time" | ||
|
|
||
| "github.com/DataDog/datadog-agent/comp/core/config" | ||
| "github.com/DataDog/datadog-agent/pkg/remoteconfig/state" | ||
| ) | ||
|
|
||
| // RemoteConfigClient defines the interface we need for remote config operations | ||
| type RemoteConfigClient interface { | ||
| GetConfigs(product string) map[string]state.RawConfig | ||
| Subscribe(product string, callback func(map[string]state.RawConfig, func(string, state.ApplyStatus))) | ||
| } | ||
|
|
||
| // Config contains information needed to create an ImageResolver | ||
| type Config struct { | ||
| Site string | ||
| DDRegistries map[string]struct{} | ||
| RCClient RemoteConfigClient | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The client ultimately shouldn't be here. The config struct should be purely config. Dependencies should be passed in when you make an image resolver
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah that makes sense. My intent here was to add it in the |
||
| MaxInitRetries int | ||
| InitRetryDelay time.Duration | ||
| } | ||
|
|
||
| // NewConfig creates a new Config | ||
| func NewConfig(cfg config.Component, rcClient RemoteConfigClient) Config { | ||
erikayasuda marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return Config{ | ||
| Site: cfg.GetString("site"), | ||
| DDRegistries: newDatadoghqRegistries(cfg.GetStringSlice("admission_controller.auto_instrumentation.default_dd_registries")), | ||
| RCClient: rcClient, | ||
| MaxInitRetries: 5, | ||
| InitRetryDelay: 1 * time.Second, | ||
|
Comment on lines
+40
to
+41
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could possibly be configured via config/env var, but this is already static in current state, so retaining that for now
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would love to see all of these configurable by
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The first two already are, and the last 2 will be removed with the pivot to tag-based, so I might not add them to |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2016-present Datadog, Inc. | ||
|
|
||
| //go:build kubeapiserver | ||
|
|
||
| package imageresolver | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/DataDog/datadog-agent/comp/core/config" | ||
| ) | ||
|
|
||
| func TestNewConfig(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| configFactory func(*testing.T) config.Component | ||
| expectedState Config | ||
| }{ | ||
| { | ||
| name: "default_config", | ||
| configFactory: func(t *testing.T) config.Component { | ||
| mockConfig := config.NewMock(t) | ||
| mockConfig.SetWithoutSource("site", "datadoghq.com") | ||
| return mockConfig | ||
| }, | ||
| expectedState: Config{ | ||
| Site: "datadoghq.com", | ||
| DDRegistries: map[string]struct{}{"gcr.io/datadoghq": {}, "docker.io/datadog": {}, "public.ecr.aws/datadog": {}}, | ||
| RCClient: nil, | ||
| MaxInitRetries: 5, | ||
| InitRetryDelay: 1 * time.Second, | ||
| }, | ||
| }, | ||
| { | ||
| name: "custom_dd_registries", | ||
| configFactory: func(t *testing.T) config.Component { | ||
| mockConfig := config.NewMock(t) | ||
| mockConfig.SetWithoutSource("site", "datadoghq.com") | ||
| mockConfig.SetWithoutSource("admission_controller.auto_instrumentation.default_dd_registries", []string{"helloworld.io/datadog"}) | ||
| return mockConfig | ||
| }, | ||
| expectedState: Config{ | ||
| Site: "datadoghq.com", | ||
| DDRegistries: map[string]struct{}{"helloworld.io/datadog": {}}, | ||
| RCClient: nil, | ||
| MaxInitRetries: 5, | ||
| InitRetryDelay: 1 * time.Second, | ||
| }, | ||
| }, | ||
| { | ||
| name: "configured_site", | ||
| configFactory: func(t *testing.T) config.Component { | ||
| mockConfig := config.NewMock(t) | ||
| mockConfig.SetWithoutSource("site", "datad0g.com") | ||
| return mockConfig | ||
| }, | ||
| expectedState: Config{ | ||
| Site: "datad0g.com", | ||
| DDRegistries: map[string]struct{}{"gcr.io/datadoghq": {}, "docker.io/datadog": {}, "public.ecr.aws/datadog": {}}, | ||
| RCClient: nil, | ||
| MaxInitRetries: 5, | ||
| InitRetryDelay: 1 * time.Second, | ||
| }, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| mockConfig := tt.configFactory(t) | ||
| result := NewConfig(mockConfig, nil) | ||
|
|
||
| require.Equal(t, tt.expectedState, result) | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // Unless explicitly stated otherwise all files in this repository are licensed | ||
| // under the Apache License Version 2.0. | ||
| // This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| // Copyright 2016-present Datadog, Inc. | ||
|
|
||
| //go:build kubeapiserver | ||
|
|
||
| // Package imageresolver provides configuration and utilities for resolving | ||
| // container image references from mutable tags to digests. | ||
| package imageresolver | ||
|
|
||
| func newDatadoghqRegistries(datadogRegistriesList []string) map[string]struct{} { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note: This is a duplicate of the one that already exists in the |
||
| datadoghqRegistries := make(map[string]struct{}) | ||
| for _, registry := range datadogRegistriesList { | ||
| datadoghqRegistries[registry] = struct{}{} | ||
| } | ||
| return datadoghqRegistries | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this nice that you only had to touch this here 👀