Skip to content

Commit 507664f

Browse files
[configgrpc, confighttp] Support lists of name/value pairs for headers (#13996)
#### Description This PR introduces: - A generic `MapList[T]` type, which is equivalent to `[]{ Name string, Value T }`, but can be unmarshalled from a `map[string]T` as well. This type is defined in a new `internal/maplist` package. - A specialized version exported as `configopaque.MapList` `configgrpc` and `confighttp` are changed to use this type instead of `map[string]configopaque.String` for storing header configuration, so as to be consistent with the SDK declarative config (see tracking issue). Note that while this is relatively elegant, it causes a number of breaking changes. An alternative would be to keep the current `map[string]configopaque.String` types, and add an Unmarshal method on the surrounding struct: see [this PoC](jade-guiton-dd@fd4c11d). #### Link to tracking issue Fixes #13930 #### Testing I added a basic unit test in `internal/maplist`. --------- Co-authored-by: Pablo Baeyens <[email protected]> Co-authored-by: Pablo Baeyens <[email protected]>
1 parent a646a2e commit 507664f

File tree

22 files changed

+427
-89
lines changed

22 files changed

+427
-89
lines changed

.chloggen/list-map-duality.yaml

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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: 'breaking'
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: all
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Change type of `configgrpc.ClientConfig.Headers`, `confighttp.ClientConfig.Headers`, and `confighttp.ServerConfig.ResponseHeaders`
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13930]
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+
`configopaque.MapList` is a new alternative to `map[string]configopaque.String` which can unmarshal
20+
both maps and lists of name/value pairs.
21+
22+
For example, if `headers` is a field of type `configopaque.MapList`,
23+
then the following YAML configs will unmarshal to the same thing:
24+
```yaml
25+
headers:
26+
"foo": "bar"
27+
28+
headers:
29+
- name: "foo"
30+
value: "bar"
31+
```
32+
33+
# Optional: The change log or logs in which this entry should be included.
34+
# e.g. '[user]' or '[user, api]'
35+
# Include 'user' if the change is relevant to end users.
36+
# Include 'api' if there is a change to a library API.
37+
# Default: '[user]'
38+
change_logs: [api]

config/configgrpc/configgrpc.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ type ClientConfig struct {
9595
WaitForReady bool `mapstructure:"wait_for_ready,omitempty"`
9696

9797
// The headers associated with gRPC requests.
98-
Headers map[string]configopaque.String `mapstructure:"headers,omitempty"`
98+
Headers configopaque.MapList `mapstructure:"headers,omitempty"`
9999

100100
// Sets the balancer in grpclb_policy to discover the servers. Default is pick_first.
101101
// https://github.com/grpc/grpc-go/blob/master/examples/features/load_balancing/README.md
@@ -289,7 +289,7 @@ func (cc *ClientConfig) ToClientConn(
289289
func (cc *ClientConfig) addHeadersIfAbsent(ctx context.Context) context.Context {
290290
kv := make([]string, 0, 2*len(cc.Headers))
291291
existingMd, _ := metadata.FromOutgoingContext(ctx)
292-
for k, v := range cc.Headers {
292+
for k, v := range cc.Headers.Iter {
293293
if len(existingMd.Get(k)) == 0 {
294294
kv = append(kv, k, string(v))
295295
}

config/configgrpc/configgrpc_test.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ func TestAllGrpcClientSettings(t *testing.T) {
163163
{
164164
name: "test all with gzip compression",
165165
settings: ClientConfig{
166-
Headers: map[string]configopaque.String{
167-
"test": "test",
166+
Headers: configopaque.MapList{
167+
{Name: "test", Value: "test"},
168168
},
169169
Endpoint: "localhost:1234",
170170
Compression: configcompression.TypeGzip,
@@ -192,8 +192,8 @@ func TestAllGrpcClientSettings(t *testing.T) {
192192
{
193193
name: "test all with snappy compression",
194194
settings: ClientConfig{
195-
Headers: map[string]configopaque.String{
196-
"test": "test",
195+
Headers: configopaque.MapList{
196+
{Name: "test", Value: "test"},
197197
},
198198
Endpoint: "localhost:1234",
199199
Compression: configcompression.TypeSnappy,
@@ -221,8 +221,8 @@ func TestAllGrpcClientSettings(t *testing.T) {
221221
{
222222
name: "test all with zstd compression",
223223
settings: ClientConfig{
224-
Headers: map[string]configopaque.String{
225-
"test": "test",
224+
Headers: configopaque.MapList{
225+
{Name: "test", Value: "test"},
226226
},
227227
Endpoint: "localhost:1234",
228228
Compression: configcompression.TypeZstd,
@@ -285,8 +285,8 @@ func TestHeaders(t *testing.T) {
285285
TLS: configtls.ClientConfig{
286286
Insecure: true,
287287
},
288-
Headers: map[string]configopaque.String{
289-
"testheader": "testvalue",
288+
Headers: configopaque.MapList{
289+
{Name: "testheader", Value: "testvalue"},
290290
},
291291
})
292292
require.NoError(t, errResp)
@@ -434,8 +434,8 @@ func TestGrpcServerAuthSettings(t *testing.T) {
434434

435435
func TestGrpcClientConfigInvalidBalancer(t *testing.T) {
436436
settings := ClientConfig{
437-
Headers: map[string]configopaque.String{
438-
"test": "test",
437+
Headers: configopaque.MapList{
438+
{Name: "test", Value: "test"},
439439
},
440440
Endpoint: "localhost:1234",
441441
Compression: "gzip",

config/confighttp/client.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ type ClientConfig struct {
5656
// Additional headers attached to each HTTP request sent by the client.
5757
// Existing header values are overwritten if collision happens.
5858
// Header values are opaque since they may be sensitive.
59-
Headers map[string]configopaque.String `mapstructure:"headers,omitempty"`
59+
Headers configopaque.MapList `mapstructure:"headers,omitempty"`
6060

6161
// Auth configuration for outgoing HTTP calls.
6262
Auth configoptional.Optional[configauth.Config] `mapstructure:"auth,omitempty"`
@@ -130,7 +130,6 @@ func NewDefaultClientConfig() ClientConfig {
130130
defaultTransport := http.DefaultTransport.(*http.Transport)
131131

132132
return ClientConfig{
133-
Headers: map[string]configopaque.String{},
134133
MaxIdleConns: defaultTransport.MaxIdleConns,
135134
IdleConnTimeout: defaultTransport.IdleConnTimeout,
136135
ForceAttemptHTTP2: true,
@@ -283,18 +282,18 @@ func (cc *ClientConfig) ToClient(ctx context.Context, host component.Host, setti
283282
// Custom RoundTripper that adds headers.
284283
type headerRoundTripper struct {
285284
transport http.RoundTripper
286-
headers map[string]configopaque.String
285+
headers configopaque.MapList
287286
}
288287

289288
// RoundTrip is a custom RoundTripper that adds headers to the request.
290289
func (interceptor *headerRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
291290
// Set Host header if provided
292-
hostHeader, found := interceptor.headers["Host"]
291+
hostHeader, found := interceptor.headers.Get("Host")
293292
if found && hostHeader != "" {
294293
// `Host` field should be set to override default `Host` header value which is Endpoint
295294
req.Host = string(hostHeader)
296295
}
297-
for k, v := range interceptor.headers {
296+
for k, v := range interceptor.headers.Iter {
298297
req.Header.Set(k, string(v))
299298
}
300299

config/confighttp/client_test.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,9 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) {
429429
settings: ClientConfig{
430430
Endpoint: "localhost:1234",
431431
Auth: configoptional.Some(configauth.Config{AuthenticatorID: mockID}),
432-
Headers: map[string]configopaque.String{"foo": "bar"},
432+
Headers: configopaque.MapList{
433+
{Name: "foo", Value: "bar"},
434+
},
433435
},
434436
shouldErr: false,
435437
host: &mockHost{
@@ -505,19 +507,19 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) {
505507
func TestHttpClientHeaders(t *testing.T) {
506508
tests := []struct {
507509
name string
508-
headers map[string]configopaque.String
510+
headers configopaque.MapList
509511
}{
510512
{
511513
name: "with_headers",
512-
headers: map[string]configopaque.String{
513-
"header1": "value1",
514+
headers: configopaque.MapList{
515+
{Name: "header1", Value: "value1"},
514516
},
515517
},
516518
}
517519
for _, tt := range tests {
518520
t.Run(tt.name, func(t *testing.T) {
519521
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
520-
for k, v := range tt.headers {
522+
for k, v := range tt.headers.Iter {
521523
assert.Equal(t, r.Header.Get(k), string(v))
522524
}
523525
w.WriteHeader(http.StatusOK)
@@ -545,11 +547,11 @@ func TestHttpClientHostHeader(t *testing.T) {
545547
hostHeader := "th"
546548
tt := struct {
547549
name string
548-
headers map[string]configopaque.String
550+
headers configopaque.MapList
549551
}{
550552
name: "with_host_header",
551-
headers: map[string]configopaque.String{
552-
"Host": configopaque.String(hostHeader),
553+
headers: configopaque.MapList{
554+
{Name: "Host", Value: configopaque.String(hostHeader)},
553555
},
554556
}
555557

@@ -724,9 +726,9 @@ func TestClientUnmarshalYAMLComprehensiveConfig(t *testing.T) {
724726
assert.Equal(t, "example.com", clientConfig.TLS.ServerName)
725727

726728
// Verify headers
727-
expectedHeaders := map[string]configopaque.String{
728-
"User-Agent": "OpenTelemetry-Collector/1.0",
729-
"X-Custom-Header": "custom-value",
729+
expectedHeaders := configopaque.MapList{
730+
{Name: "User-Agent", Value: "OpenTelemetry-Collector/1.0"},
731+
{Name: "X-Custom-Header", Value: "custom-value"},
730732
}
731733
assert.Equal(t, expectedHeaders, clientConfig.Headers)
732734

config/confighttp/server.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ type ServerConfig struct {
5252

5353
// Additional headers attached to each HTTP response sent to the client.
5454
// Header values are opaque since they may be sensitive.
55-
ResponseHeaders map[string]configopaque.String `mapstructure:"response_headers"`
55+
ResponseHeaders configopaque.MapList `mapstructure:"response_headers,omitempty"`
5656

5757
// CompressionAlgorithms configures the list of compression algorithms the server can accept. Default: ["", "gzip", "zstd", "zlib", "snappy", "deflate"]
5858
CompressionAlgorithms []string `mapstructure:"compression_algorithms,omitempty"`
@@ -102,7 +102,6 @@ type ServerConfig struct {
102102
// We encourage to use this function to create an object of ServerConfig.
103103
func NewDefaultServerConfig() ServerConfig {
104104
return ServerConfig{
105-
ResponseHeaders: map[string]configopaque.String{},
106105
WriteTimeout: 30 * time.Second,
107106
ReadHeaderTimeout: 1 * time.Minute,
108107
IdleTimeout: 1 * time.Minute,
@@ -287,11 +286,11 @@ func (sc *ServerConfig) ToServer(ctx context.Context, host component.Host, setti
287286
return server, err
288287
}
289288

290-
func responseHeadersHandler(handler http.Handler, headers map[string]configopaque.String) http.Handler {
289+
func responseHeadersHandler(handler http.Handler, headers configopaque.MapList) http.Handler {
291290
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
292291
h := w.Header()
293292

294-
for k, v := range headers {
293+
for k, v := range headers.Iter {
295294
h.Set(k, string(v))
296295
}
297296

config/confighttp/server_test.go

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -428,21 +428,17 @@ func TestHttpCorsWithSettings(t *testing.T) {
428428
func TestHttpServerHeaders(t *testing.T) {
429429
tests := []struct {
430430
name string
431-
headers map[string]configopaque.String
431+
headers configopaque.MapList
432432
}{
433433
{
434434
name: "noHeaders",
435435
headers: nil,
436436
},
437-
{
438-
name: "emptyHeaders",
439-
headers: map[string]configopaque.String{},
440-
},
441437
{
442438
name: "withHeaders",
443-
headers: map[string]configopaque.String{
444-
"x-new-header-1": "value1",
445-
"x-new-header-2": "value2",
439+
headers: configopaque.MapList{
440+
{Name: "x-new-header-1", Value: "value1"},
441+
{Name: "x-new-header-2", Value: "value2"},
446442
},
447443
},
448444
}
@@ -515,7 +511,7 @@ func verifyCorsResp(t *testing.T, url, origin string, set configoptional.Optiona
515511
assert.Equal(t, wantMaxAge, resp.Header.Get("Access-Control-Max-Age"))
516512
}
517513

518-
func verifyHeadersResp(t *testing.T, url string, expected map[string]configopaque.String) {
514+
func verifyHeadersResp(t *testing.T, url string, expected configopaque.MapList) {
519515
req, err := http.NewRequest(http.MethodGet, url, http.NoBody)
520516
require.NoError(t, err, "Error creating request")
521517

@@ -526,7 +522,7 @@ func verifyHeadersResp(t *testing.T, url string, expected map[string]configopaqu
526522

527523
assert.Equal(t, http.StatusOK, resp.StatusCode)
528524

529-
for k, v := range expected {
525+
for k, v := range expected.Iter {
530526
assert.Equal(t, string(v), resp.Header.Get(k))
531527
}
532528
}
@@ -950,7 +946,6 @@ func BenchmarkHttpRequest(b *testing.B) {
950946

951947
func TestDefaultHTTPServerSettings(t *testing.T) {
952948
httpServerSettings := NewDefaultServerConfig()
953-
assert.NotNil(t, httpServerSettings.ResponseHeaders)
954949
assert.NotNil(t, httpServerSettings.CORS)
955950
assert.NotNil(t, httpServerSettings.TLS)
956951
assert.Equal(t, 1*time.Minute, httpServerSettings.IdleTimeout)
@@ -1138,9 +1133,9 @@ func TestServerUnmarshalYAMLComprehensiveConfig(t *testing.T) {
11381133
assert.Equal(t, 7200, serverConfig.CORS.Get().MaxAge)
11391134

11401135
// Verify response headers
1141-
expectedResponseHeaders := map[string]configopaque.String{
1142-
"Server": "OpenTelemetry-Collector",
1143-
"X-Flavor": "apple",
1136+
expectedResponseHeaders := configopaque.MapList{
1137+
{Name: "Server", Value: "OpenTelemetry-Collector"},
1138+
{Name: "X-Flavor", Value: "apple"},
11441139
}
11451140
assert.Equal(t, expectedResponseHeaders, serverConfig.ResponseHeaders)
11461141

config/configopaque/go.mod

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,31 @@ go 1.24.0
44

55
require (
66
github.com/stretchr/testify v1.11.1
7+
go.opentelemetry.io/collector/confmap v1.44.0
8+
go.opentelemetry.io/collector/confmap/xconfmap v0.138.0
79
go.uber.org/goleak v1.3.0
810
go.yaml.in/yaml/v3 v3.0.4
911
)
1012

1113
require (
1214
github.com/davecgh/go-spew v1.1.1 // indirect
13-
github.com/kr/pretty v0.3.1 // indirect
15+
github.com/go-viper/mapstructure/v2 v2.4.0 // indirect
16+
github.com/gobwas/glob v0.2.3 // indirect
17+
github.com/hashicorp/go-version v1.7.0 // indirect
18+
github.com/knadh/koanf/maps v0.1.2 // indirect
19+
github.com/knadh/koanf/providers/confmap v1.0.0 // indirect
20+
github.com/knadh/koanf/v2 v2.3.0 // indirect
21+
github.com/mitchellh/copystructure v1.2.0 // indirect
22+
github.com/mitchellh/reflectwalk v1.0.2 // indirect
1423
github.com/pmezard/go-difflib v1.0.0 // indirect
15-
github.com/rogpeppe/go-internal v1.10.0 // indirect
16-
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect
24+
go.opentelemetry.io/collector/featuregate v1.44.0 // indirect
25+
go.uber.org/multierr v1.11.0 // indirect
26+
go.uber.org/zap v1.27.0 // indirect
1727
gopkg.in/yaml.v3 v3.0.1 // indirect
1828
)
29+
30+
replace go.opentelemetry.io/collector/featuregate => ../../featuregate
31+
32+
replace go.opentelemetry.io/collector/confmap => ../../confmap
33+
34+
replace go.opentelemetry.io/collector/confmap/xconfmap => ../../confmap/xconfmap

config/configopaque/go.sum

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

0 commit comments

Comments
 (0)