Skip to content

Commit 3d86a5c

Browse files
erikburtkrajorama
andauthored
[receiver/prometheusreceiver] fix prom receiver config reload (#40780)
#### Description Fixing a bug where reloading the Prometheus config, caused config entries of type [Secret](https://github.com/prometheus/common/blob/95acce133ca2c07a966a71d475fb936fc282db18/config/config.go#L36-L44) to be replaced with the literal value of `<secret>`. - Switches to new yaml library which allows for custom marshaling (shoutout to @ridwanmsharif) - Adds unit test to ensure reloading promreceiver config doesn't mask secrets #### Link to tracking issue Fixes #40520, #40916 #### Testing - I've added a unit test, validating the `Reload()` method doesn't mask secrets for existing `scrape_configs` - Pointed a local instance at a simple http server to ensure the credentials are not affected (#40520 (comment)) - Tested locally it with a `target_allocator` config, which was the original reason for the `Reload()` method, which introduced the bug (https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/40103/files#r2097520578) #### Documentation N/A --------- Co-authored-by: George Krajcsovits <[email protected]>
1 parent cea83aa commit 3d86a5c

File tree

24 files changed

+152
-13
lines changed

24 files changed

+152
-13
lines changed

.chloggen/fix_prom-receiver-auth.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: receiver/prometheusreceiver
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Fixes masking of authentication credentials in Prometheus receiver, when reloading the Prometheus config."
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [40520, 40916]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

connector/datadogconnector/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

exporter/datadogexporter/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ require (
238238
github.com/go-zookeeper/zk v1.0.4 // indirect
239239
github.com/gobwas/glob v0.2.3 // indirect
240240
github.com/goccy/go-json v0.10.5 // indirect
241+
github.com/goccy/go-yaml v1.18.0 // indirect
241242
github.com/gofrs/flock v0.12.1 // indirect
242243
github.com/gogo/protobuf v1.3.2 // indirect
243244
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect

exporter/datadogexporter/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

exporter/datadogexporter/integrationtest/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

exporter/prometheusexporter/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ require (
8686
github.com/go-viper/mapstructure/v2 v2.3.0 // indirect
8787
github.com/go-zookeeper/zk v1.0.4 // indirect
8888
github.com/gobwas/glob v0.2.3 // indirect
89+
github.com/goccy/go-yaml v1.18.0 // indirect
8990
github.com/gogo/protobuf v1.3.2 // indirect
9091
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
9192
github.com/golang/protobuf v1.5.4 // indirect

exporter/prometheusexporter/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

receiver/prometheusreceiver/config.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ import (
1111
"sort"
1212
"strings"
1313

14+
"github.com/goccy/go-yaml"
1415
commonconfig "github.com/prometheus/common/config"
1516
promconfig "github.com/prometheus/prometheus/config"
1617
"github.com/prometheus/prometheus/discovery/kubernetes"
1718
"go.opentelemetry.io/collector/config/confighttp"
1819
"go.opentelemetry.io/collector/confmap"
19-
"gopkg.in/yaml.v3"
2020

2121
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/targetallocator"
2222
)
@@ -145,10 +145,16 @@ func (cfg *PromConfig) Validate() error {
145145
}
146146

147147
func reloadPromConfig(dst *PromConfig, src any) error {
148-
yamlOut, err := yaml.Marshal(src)
148+
yamlOut, err := yaml.MarshalWithOptions(
149+
src,
150+
yaml.CustomMarshaler(func(s commonconfig.Secret) ([]byte, error) {
151+
return []byte(s), nil
152+
}),
153+
)
149154
if err != nil {
150155
return fmt.Errorf("prometheus receiver: failed to marshal config to yaml: %w", err)
151156
}
157+
152158
newCfg, err := promconfig.Load(string(yamlOut), slog.Default())
153159
if err != nil {
154160
return fmt.Errorf("prometheus receiver: failed to unmarshal yaml to prometheus config object: %w", err)

receiver/prometheusreceiver/config_test.go

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,16 @@ package prometheusreceiver
55

66
import (
77
"context"
8+
"log/slog"
9+
"os"
810
"path/filepath"
911
"strings"
1012
"testing"
1113
"time"
1214

1315
promConfig "github.com/prometheus/common/config"
1416
promModel "github.com/prometheus/common/model"
17+
promconfig "github.com/prometheus/prometheus/config"
1518
"github.com/stretchr/testify/assert"
1619
"github.com/stretchr/testify/require"
1720
"go.opentelemetry.io/collector/component"
@@ -399,3 +402,79 @@ func TestLoadPrometheusAPIServerExtensionConfig(t *testing.T) {
399402
require.NoError(t, sub.Unmarshal(cfg))
400403
require.Error(t, xconfmap.Validate(cfg))
401404
}
405+
406+
func TestReloadPromConfigSecretHandling(t *testing.T) {
407+
// This test verifies that the Reload() method preserves secrets instead of
408+
// corrupting them to "<secret>" placeholders. This is critical for authentication
409+
// to work properly when using configurations with basic auth or bearer tokens.
410+
411+
tests := []struct {
412+
name string
413+
configYAML string
414+
checkFn func(t *testing.T, dst *PromConfig)
415+
}{
416+
{
417+
name: "basic auth password preservation",
418+
configYAML: `
419+
scrape_configs:
420+
- job_name: "test-basic-auth"
421+
basic_auth:
422+
username: "testuser"
423+
password: "mysecretpassword"
424+
static_configs:
425+
- targets: ["localhost:8080"]
426+
`,
427+
checkFn: func(t *testing.T, dst *PromConfig) {
428+
require.Len(t, dst.ScrapeConfigs, 1)
429+
scrapeConfig := dst.ScrapeConfigs[0]
430+
assert.Equal(t, "test-basic-auth", scrapeConfig.JobName)
431+
432+
// The critical check: ensure the password is not "<secret>"
433+
require.NotNil(t, scrapeConfig.HTTPClientConfig.BasicAuth, "basic auth should be configured")
434+
password := string(scrapeConfig.HTTPClientConfig.BasicAuth.Password)
435+
assert.Equal(t, "mysecretpassword", password, "password should preserve original value")
436+
assert.Equal(t, "testuser", scrapeConfig.HTTPClientConfig.BasicAuth.Username)
437+
},
438+
},
439+
{
440+
name: "bearer token preservation",
441+
configYAML: `
442+
scrape_configs:
443+
- job_name: "test-bearer-token"
444+
authorization:
445+
type: "Bearer"
446+
credentials: "mySecretBearerToken123"
447+
static_configs:
448+
- targets: ["localhost:9090"]
449+
`,
450+
checkFn: func(t *testing.T, dst *PromConfig) {
451+
require.Len(t, dst.ScrapeConfigs, 1)
452+
scrapeConfig := dst.ScrapeConfigs[0]
453+
assert.Equal(t, "test-bearer-token", scrapeConfig.JobName)
454+
455+
// Check that bearer token is preserved
456+
require.NotNil(t, scrapeConfig.HTTPClientConfig.Authorization, "authorization should be configured")
457+
credentials := string(scrapeConfig.HTTPClientConfig.Authorization.Credentials)
458+
assert.Equal(t, "mySecretBearerToken123", credentials, "credentials should preserve original value")
459+
assert.Equal(t, "Bearer", scrapeConfig.HTTPClientConfig.Authorization.Type)
460+
},
461+
},
462+
}
463+
464+
for _, tt := range tests {
465+
t.Run(tt.name, func(t *testing.T) {
466+
// Load the config using promconfig.Load to simulate real usage
467+
initialCfg, err := promconfig.Load(tt.configYAML, slog.New(slog.NewTextHandler(os.Stderr, nil)))
468+
require.NoError(t, err)
469+
470+
// Convert to PromConfig and test the Reload method
471+
// The Reload method should preserve secrets and not corrupt them
472+
dst := (*PromConfig)(initialCfg)
473+
err = dst.Reload()
474+
require.NoError(t, err)
475+
476+
// Verify that secrets are preserved
477+
tt.checkFn(t, dst)
478+
})
479+
}
480+
}

receiver/prometheusreceiver/go.mod

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go 1.23.0
44

55
require (
66
github.com/go-kit/log v0.2.1
7+
github.com/goccy/go-yaml v1.18.0
78
github.com/gogo/protobuf v1.3.2
89
github.com/golang/snappy v1.0.0
910
github.com/grafana/regexp v0.0.0-20240518133315-a468a5bfb3bc
@@ -44,7 +45,6 @@ require (
4445
go.uber.org/zap/exp v0.3.0
4546
golang.org/x/net v0.41.0
4647
google.golang.org/protobuf v1.36.6
47-
gopkg.in/yaml.v3 v3.0.1
4848
)
4949

5050
require (
@@ -281,6 +281,7 @@ require (
281281
gopkg.in/inf.v0 v0.9.1 // indirect
282282
gopkg.in/ini.v1 v1.67.0 // indirect
283283
gopkg.in/yaml.v2 v2.4.0 // indirect
284+
gopkg.in/yaml.v3 v3.0.1 // indirect
284285
k8s.io/api v0.32.3 // indirect
285286
k8s.io/apimachinery v0.32.3 // indirect
286287
k8s.io/client-go v0.32.3 // indirect

receiver/prometheusreceiver/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

receiver/prometheusreceiver/metrics_receiver_helper_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"testing"
2020
"time"
2121

22+
"github.com/goccy/go-yaml"
2223
"github.com/gogo/protobuf/proto"
2324
"github.com/prometheus/common/promslog"
2425
promcfg "github.com/prometheus/prometheus/config"
@@ -34,7 +35,6 @@ import (
3435
"go.opentelemetry.io/collector/pdata/pmetric"
3536
"go.opentelemetry.io/collector/receiver/receivertest"
3637
semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
37-
"gopkg.in/yaml.v3"
3838

3939
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/internal"
4040
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/internal/metadata"

receiver/prometheusreceiver/metrics_receiver_scrape_config_files_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@ import (
77
"os"
88
"testing"
99

10+
"github.com/goccy/go-yaml"
1011
"github.com/prometheus/prometheus/config"
1112
"github.com/stretchr/testify/assert"
1213
"github.com/stretchr/testify/require"
1314
"go.opentelemetry.io/collector/pdata/pmetric"
1415
semconv "go.opentelemetry.io/otel/semconv/v1.27.0"
15-
"gopkg.in/yaml.v3"
1616
)
1717

1818
var scrapeFileTargetPage = `

receiver/prometheusreceiver/metrics_receiver_target_allocator_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"testing"
1616
"time"
1717

18+
"github.com/goccy/go-yaml"
1819
promTestUtil "github.com/prometheus/client_golang/prometheus/testutil"
1920
"github.com/prometheus/common/model"
2021
"github.com/prometheus/common/promslog"
@@ -28,7 +29,6 @@ import (
2829
"go.opentelemetry.io/collector/receiver/receivertest"
2930
"go.uber.org/zap"
3031
"go.uber.org/zap/zapcore"
31-
"gopkg.in/yaml.v3"
3232

3333
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/internal/metadata"
3434
"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/targetallocator"

receiver/prometheusreceiver/targetallocator/config.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package targetallocator // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/prometheusreceiver/targetallocator"
55

66
import (
7-
"bytes"
87
"encoding/base64"
98
"errors"
109
"fmt"
@@ -13,11 +12,11 @@ import (
1312
"strings"
1413
"time"
1514

15+
"github.com/goccy/go-yaml"
1616
commonconfig "github.com/prometheus/common/config"
1717
promHTTP "github.com/prometheus/prometheus/discovery/http"
1818
"go.opentelemetry.io/collector/config/confighttp"
1919
"go.opentelemetry.io/collector/confmap"
20-
"gopkg.in/yaml.v3"
2120
)
2221

2322
type Config struct {
@@ -111,14 +110,17 @@ func checkTLSConfig(tlsConfig commonconfig.TLSConfig) error {
111110
}
112111

113112
func unmarshalYAML(in map[string]any, out any) error {
114-
yamlOut, err := yaml.Marshal(in)
113+
yamlOut, err := yaml.MarshalWithOptions(
114+
in,
115+
yaml.CustomMarshaler[commonconfig.Secret](func(s commonconfig.Secret) ([]byte, error) {
116+
return []byte(s), nil
117+
}),
118+
)
115119
if err != nil {
116120
return fmt.Errorf("prometheus receiver: failed to marshal config to yaml: %w", err)
117121
}
118122

119-
decoder := yaml.NewDecoder(bytes.NewReader(yamlOut))
120-
decoder.KnownFields(true)
121-
err = decoder.Decode(out)
123+
err = yaml.Unmarshal(yamlOut, out)
122124
if err != nil {
123125
return fmt.Errorf("prometheus receiver: failed to unmarshal yaml to prometheus config object: %w", err)
124126
}

receiver/prometheusreceiver/targetallocator/manager.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"sort"
1616
"time"
1717

18+
"github.com/goccy/go-yaml"
1819
commonconfig "github.com/prometheus/common/config"
1920
"github.com/prometheus/common/model"
2021
promconfig "github.com/prometheus/prometheus/config"
@@ -24,7 +25,6 @@ import (
2425
"go.opentelemetry.io/collector/component"
2526
"go.opentelemetry.io/collector/receiver"
2627
"go.uber.org/zap"
27-
"gopkg.in/yaml.v3"
2828
)
2929

3030
type Manager struct {

receiver/purefareceiver/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ require (
8080
github.com/go-viper/mapstructure/v2 v2.3.0 // indirect
8181
github.com/go-zookeeper/zk v1.0.4 // indirect
8282
github.com/gobwas/glob v0.2.3 // indirect
83+
github.com/goccy/go-yaml v1.18.0 // indirect
8384
github.com/gogo/protobuf v1.3.2 // indirect
8485
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
8586
github.com/golang/protobuf v1.5.4 // indirect

receiver/purefareceiver/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

receiver/purefbreceiver/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ require (
7979
github.com/go-viper/mapstructure/v2 v2.3.0 // indirect
8080
github.com/go-zookeeper/zk v1.0.4 // indirect
8181
github.com/gobwas/glob v0.2.3 // indirect
82+
github.com/goccy/go-yaml v1.18.0 // indirect
8283
github.com/gogo/protobuf v1.3.2 // indirect
8384
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
8485
github.com/golang/protobuf v1.5.4 // indirect

receiver/purefbreceiver/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

receiver/simpleprometheusreceiver/go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ require (
7676
github.com/go-viper/mapstructure/v2 v2.3.0 // indirect
7777
github.com/go-zookeeper/zk v1.0.4 // indirect
7878
github.com/gobwas/glob v0.2.3 // indirect
79+
github.com/goccy/go-yaml v1.18.0 // indirect
7980
github.com/gogo/protobuf v1.3.2 // indirect
8081
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
8182
github.com/golang/protobuf v1.5.4 // indirect

receiver/simpleprometheusreceiver/go.sum

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)