From 87cf432dd55841f0b369b2457256ead5e88a890e Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Thu, 4 Dec 2025 19:45:42 +0100 Subject: [PATCH 1/2] [chore] prometheusreceiver: Refactor and remove dead code Refactor PromHTTPSDConfig.Unmarshal and PromHTTPClientConfig.Unmarshal so they call a more specific function unmarshalConf, that knows it won't have to YAML marshal commonconfig.Secret instances. Signed-off-by: Arve Knudsen --- .../internal/targetallocator/config.go | 38 ++++++++------- .../internal/targetallocator/config_test.go | 48 +++++++++++++++++++ 2 files changed, 68 insertions(+), 18 deletions(-) diff --git a/receiver/prometheusreceiver/internal/targetallocator/config.go b/receiver/prometheusreceiver/internal/targetallocator/config.go index 5a7a295ed9a5a..8d78eef27cf70 100644 --- a/receiver/prometheusreceiver/internal/targetallocator/config.go +++ b/receiver/prometheusreceiver/internal/targetallocator/config.go @@ -47,12 +47,10 @@ func (cfg *Config) Validate() error { var _ confmap.Unmarshaler = (*PromHTTPSDConfig)(nil) func (cfg *PromHTTPSDConfig) Unmarshal(componentParser *confmap.Conf) error { - cfgMap := componentParser.ToStringMap() - if len(cfgMap) == 0 { - return nil - } - cfgMap["url"] = "http://placeholder" // we have to set it as else marshaling will fail - return unmarshalYAML(cfgMap, (*promHTTP.SDConfig)(cfg)) + return unmarshalConf(componentParser, func(m map[string]any) { + // we have to set it as else marshaling will fail + m["url"] = "http://placeholder" + }, (*promHTTP.SDConfig)(cfg)) } type PromHTTPClientConfig commonconfig.HTTPClientConfig @@ -60,11 +58,7 @@ type PromHTTPClientConfig commonconfig.HTTPClientConfig var _ confmap.Unmarshaler = (*PromHTTPClientConfig)(nil) func (cfg *PromHTTPClientConfig) Unmarshal(componentParser *confmap.Conf) error { - cfgMap := componentParser.ToStringMap() - if len(cfgMap) == 0 { - return nil - } - return unmarshalYAML(cfgMap, (*commonconfig.HTTPClientConfig)(cfg)) + return unmarshalConf(componentParser, nil, (*commonconfig.HTTPClientConfig)(cfg)) } func (cfg *PromHTTPClientConfig) Validate() error { @@ -109,13 +103,21 @@ func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error { return nil } -func unmarshalYAML(in map[string]any, out any) error { - yamlOut, err := yaml.MarshalWithOptions( - in, - yaml.CustomMarshaler[commonconfig.Secret](func(s commonconfig.Secret) ([]byte, error) { - return []byte(s), nil - }), - ) +// unmarshalConf unmarshals conf to out. +// YAML is used as an intermediary format, because confmap.Conf.Unmarshal is incompatible with Prometheus types, +// as the former uses mapstructure tags . +// If cb is not nil, it's called to mutate the map representation of conf. +func unmarshalConf(conf *confmap.Conf, cb func(map[string]any), out any) error { + in := conf.ToStringMap() + if len(in) == 0 { + return nil + } + + if cb != nil { + cb(in) + } + + yamlOut, err := yaml.Marshal(in) if err != nil { return fmt.Errorf("prometheus receiver: failed to marshal config to yaml: %w", err) } diff --git a/receiver/prometheusreceiver/internal/targetallocator/config_test.go b/receiver/prometheusreceiver/internal/targetallocator/config_test.go index b64a356768401..91ed4d639dd31 100644 --- a/receiver/prometheusreceiver/internal/targetallocator/config_test.go +++ b/receiver/prometheusreceiver/internal/targetallocator/config_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/collector/component/componenttest" + "go.opentelemetry.io/collector/confmap" "go.opentelemetry.io/collector/confmap/confmaptest" "go.opentelemetry.io/collector/confmap/xconfmap" ) @@ -228,3 +229,50 @@ func TestCheckTLSConfig(t *testing.T) { }) } } + +func TestUnmarshalConf(t *testing.T) { + t.Run("empty config", func(t *testing.T) { + var cfg promConfig.HTTPClientConfig + err := unmarshalConf(confmap.NewFromStringMap(map[string]any{}), nil, &cfg) + require.NoError(t, err) + assert.Zero(t, cfg) + }) + + t.Run("special YAML characters preserved", func(t *testing.T) { + var cfg promConfig.HTTPClientConfig + input := map[string]any{ + "basic_auth": map[string]any{ + "username": "user", + "password": "%password-with-percent", + }, + } + err := unmarshalConf(confmap.NewFromStringMap(input), nil, &cfg) + require.NoError(t, err) + require.NotNil(t, cfg.BasicAuth) + assert.Equal(t, "%password-with-percent", string(cfg.BasicAuth.Password)) + }) + + t.Run("callback mutates config", func(t *testing.T) { + var cfg promConfig.HTTPClientConfig + input := map[string]any{ + "basic_auth": map[string]any{ + "username": "original", + }, + } + cb := func(m map[string]any) { + m["basic_auth"].(map[string]any)["username"] = "mutated" + } + require.NoError(t, unmarshalConf(confmap.NewFromStringMap(input), cb, &cfg)) + require.NotNil(t, cfg.BasicAuth) + assert.Equal(t, "mutated", cfg.BasicAuth.Username) + }) + + t.Run("marshal error", func(t *testing.T) { + var cfg promConfig.HTTPClientConfig + input := map[string]any{ + "invalid": make(chan int), // channels can't be marshaled to YAML + } + err := unmarshalConf(confmap.NewFromStringMap(input), nil, &cfg) + require.ErrorContains(t, err, "failed to marshal") + }) +} From 433ab8d35a065c93fb801260453fe5a8023756b1 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Tue, 9 Dec 2025 17:52:22 +0100 Subject: [PATCH 2/2] Add integration test Signed-off-by: Arve Knudsen --- .../internal/targetallocator/config_test.go | 43 +++++++++++++------ .../testdata/config_with_special_chars.yaml | 10 +++++ 2 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 receiver/prometheusreceiver/internal/targetallocator/testdata/config_with_special_chars.yaml diff --git a/receiver/prometheusreceiver/internal/targetallocator/config_test.go b/receiver/prometheusreceiver/internal/targetallocator/config_test.go index c10037763ed78..58c5ad62f1183 100644 --- a/receiver/prometheusreceiver/internal/targetallocator/config_test.go +++ b/receiver/prometheusreceiver/internal/targetallocator/config_test.go @@ -25,20 +25,39 @@ func TestComponentConfigStruct(t *testing.T) { } func TestLoadTargetAllocatorConfig(t *testing.T) { - cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) - require.NoError(t, err) - cfg := &Config{} + t.Run("basic", func(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml")) + require.NoError(t, err) + cfg := &Config{} - sub, err := cm.Sub("target_allocator") - require.NoError(t, err) - require.NoError(t, sub.Unmarshal(cfg)) - require.NoError(t, xconfmap.Validate(cfg)) + sub, err := cm.Sub("target_allocator") + require.NoError(t, err) + require.NoError(t, sub.Unmarshal(cfg)) + require.NoError(t, xconfmap.Validate(cfg)) + + assert.Equal(t, "http://localhost:8080", cfg.Endpoint) + assert.Equal(t, 5*time.Second, cfg.Timeout) + assert.Equal(t, "client.crt", cfg.TLS.CertFile) + assert.Equal(t, 30*time.Second, cfg.Interval) + assert.Equal(t, "collector-1", cfg.CollectorID) + }) - assert.Equal(t, "http://localhost:8080", cfg.Endpoint) - assert.Equal(t, 5*time.Second, cfg.Timeout) - assert.Equal(t, "client.crt", cfg.TLS.CertFile) - assert.Equal(t, 30*time.Second, cfg.Interval) - assert.Equal(t, "collector-1", cfg.CollectorID) + t.Run("special characters in password", func(t *testing.T) { + cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config_with_special_chars.yaml")) + require.NoError(t, err) + + var cfg Config + sub, err := cm.Sub("target_allocator") + require.NoError(t, err) + require.NoError(t, sub.Unmarshal(&cfg)) + + require.NotNil(t, cfg.HTTPSDConfig, "http_sd_config should be present") + require.NotNil(t, cfg.HTTPScrapeConfig, "http_scrape_config should be present") + require.NotNil(t, cfg.HTTPScrapeConfig.BasicAuth, "basic_auth should be present") + assert.Equal(t, "testuser", cfg.HTTPScrapeConfig.BasicAuth.Username) + assert.Equal(t, "%password-with-percent", string(cfg.HTTPScrapeConfig.BasicAuth.Password), + "password with special YAML characters should be preserved") + }) } func TestPromHTTPClientConfigValidateAuthorization(t *testing.T) { diff --git a/receiver/prometheusreceiver/internal/targetallocator/testdata/config_with_special_chars.yaml b/receiver/prometheusreceiver/internal/targetallocator/testdata/config_with_special_chars.yaml new file mode 100644 index 0000000000000..c705da528ae23 --- /dev/null +++ b/receiver/prometheusreceiver/internal/targetallocator/testdata/config_with_special_chars.yaml @@ -0,0 +1,10 @@ +target_allocator: + endpoint: http://localhost:8080 + interval: 30s + collector_id: collector-1 + http_sd_config: + refresh_interval: 60s + http_scrape_config: + basic_auth: + username: testuser + password: "%password-with-percent"