Skip to content

Commit 73f090c

Browse files
[confighttp, configgrpc] Replace host parameter by map of extensions (#14190)
#### Description Alternative to #14162, where we make the map of extensions a mandatory positional parameter to replace `host` instead of an option. #### Link to tracking issue Fixes #13640 #### Testing Existing tests make extensive use of the modified functions #### Documentation Added line in ToServer/ToClient(Conn) documentation describing the intended use of the extensions parameter. --------- Co-authored-by: Pablo Baeyens <[email protected]> Co-authored-by: Pablo Baeyens <[email protected]>
1 parent 974da01 commit 73f090c

File tree

22 files changed

+304
-302
lines changed

22 files changed

+304
-302
lines changed
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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: pkg/config/configgrpc
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Replace `component.Host` parameter of ToServer/ToClientConn by map of extensions
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13640]
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+
Components must now pass the map obtained from the host's `GetExtensions` method
20+
instead of the host itself.
21+
22+
Nil may be used in tests where no middleware or authentication extensions are used.
23+
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: [api]
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
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: pkg/config/confighttp
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Replace `component.Host` parameter of ToServer/ToClient by map of extensions
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [13640]
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+
Components must now pass the map obtained from the host's `GetExtensions` method
20+
instead of the host itself.
21+
22+
Nil may be used in tests where no middleware or authentication extensions are used.
23+
24+
# Optional: The change log or logs in which this entry should be included.
25+
# e.g. '[user]' or '[user, api]'
26+
# Include 'user' if the change is relevant to end users.
27+
# Include 'api' if there is a change to a library API.
28+
# Default: '[user]'
29+
change_logs: [api]

component/componenttest/nop_host.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ func NewNopHost() component.Host {
1919
return &nopHost{}
2020
}
2121

22-
// GetExtensions returns a `nil` extensions map.
22+
// GetExtensions returns an empty extensions map.
2323
func (nh *nopHost) GetExtensions() map[component.ID]component.Component {
24-
return nil
24+
return map[component.ID]component.Component{}
2525
}

component/componenttest/nop_host_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,5 +15,7 @@ func TestNewNopHost(t *testing.T) {
1515
require.NotNil(t, nh)
1616
require.IsType(t, &nopHost{}, nh)
1717

18-
assert.Nil(t, nh.GetExtensions())
18+
extensions := nh.GetExtensions()
19+
assert.NotNil(t, extensions)
20+
assert.Empty(t, extensions)
1921
}

component/host.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,7 @@ type Host interface {
1717
//
1818
// GetExtensions can be called by the component anytime after Component.Start() begins and
1919
// until Component.Shutdown() ends.
20+
//
21+
// The returned map should only be nil if the host does not support extensions at all.
2022
GetExtensions() map[ID]Component
2123
}

config/configgrpc/client_middleware_test.go

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,8 @@ func TestClientMiddlewareOrdering(t *testing.T) {
124124
},
125125
}
126126

127-
// Create a test host with our mock extensions
128-
host := &mockHost{ext: mockExt}
129-
130127
// Send a request using the client with middleware
131-
resp, err := sendTestRequestWithHost(t, clientConfig, host)
128+
resp, err := sendTestRequestWithExtensions(t, clientConfig, mockExt)
132129
require.NoError(t, err)
133130
assert.NotNil(t, resp)
134131

@@ -147,16 +144,14 @@ func TestClientMiddlewareOrdering(t *testing.T) {
147144
// specifically related to middleware resolution and API calls.
148145
func TestClientMiddlewareToClientErrors(t *testing.T) {
149146
tests := []struct {
150-
name string
151-
host component.Host
152-
config ClientConfig
153-
errText string
147+
name string
148+
extensions map[component.ID]component.Component
149+
config ClientConfig
150+
errText string
154151
}{
155152
{
156-
name: "extension_not_found",
157-
host: &mockHost{
158-
ext: map[component.ID]component.Component{},
159-
},
153+
name: "extension_not_found",
154+
extensions: map[component.ID]component.Component{},
160155
config: ClientConfig{
161156
Endpoint: "localhost:1234",
162157
TLS: configtls.ClientConfig{
@@ -172,10 +167,8 @@ func TestClientMiddlewareToClientErrors(t *testing.T) {
172167
},
173168
{
174169
name: "get_client_options_fails",
175-
host: &mockHost{
176-
ext: map[component.ID]component.Component{
177-
component.MustNewID("errormw"): extensionmiddlewaretest.NewErr(errors.New("get options failed")),
178-
},
170+
extensions: map[component.ID]component.Component{
171+
component.MustNewID("errormw"): extensionmiddlewaretest.NewErr(errors.New("get options failed")),
179172
},
180173
config: ClientConfig{
181174
Endpoint: "localhost:1234",
@@ -195,7 +188,7 @@ func TestClientMiddlewareToClientErrors(t *testing.T) {
195188
for _, tc := range tests {
196189
t.Run(tc.name, func(t *testing.T) {
197190
// Test creating the client with middleware errors
198-
_, err := tc.config.ToClientConn(context.Background(), tc.host, componenttest.NewNopTelemetrySettings())
191+
_, err := tc.config.ToClientConn(context.Background(), tc.extensions, componenttest.NewNopTelemetrySettings())
199192
require.Error(t, err)
200193
assert.Contains(t, err.Error(), tc.errText)
201194
})

config/configgrpc/configgrpc.go

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -272,13 +272,17 @@ func (grpcDialOptionWrapper) isToClientConnOption() {}
272272
// a non-blocking dial (the function won't wait for connections to be
273273
// established, and connecting happens in the background). To make it a blocking
274274
// dial, use the WithGrpcDialOption(grpc.WithBlock()) option.
275+
//
276+
// To allow the configuration to reference middleware or authentication extensions,
277+
// the `extensions` argument should be the output of `host.GetExtensions()`.
278+
// It may also be `nil` in tests where no such extension is expected to be used.
275279
func (cc *ClientConfig) ToClientConn(
276280
ctx context.Context,
277-
host component.Host,
281+
extensions map[component.ID]component.Component,
278282
settings component.TelemetrySettings,
279283
extraOpts ...ToClientConnOption,
280284
) (*grpc.ClientConn, error) {
281-
grpcOpts, err := cc.getGrpcDialOptions(ctx, host, settings, extraOpts)
285+
grpcOpts, err := cc.getGrpcDialOptions(ctx, extensions, settings, extraOpts)
282286
if err != nil {
283287
return nil, err
284288
}
@@ -299,7 +303,7 @@ func (cc *ClientConfig) addHeadersIfAbsent(ctx context.Context) context.Context
299303

300304
func (cc *ClientConfig) getGrpcDialOptions(
301305
ctx context.Context,
302-
host component.Host,
306+
extensions map[component.ID]component.Component,
303307
settings component.TelemetrySettings,
304308
extraOpts []ToClientConnOption,
305309
) ([]grpc.DialOption, error) {
@@ -343,11 +347,11 @@ func (cc *ClientConfig) getGrpcDialOptions(
343347
}
344348

345349
if cc.Auth.HasValue() {
346-
if host.GetExtensions() == nil {
347-
return nil, errors.New("no extensions configuration available")
350+
if extensions == nil {
351+
return nil, errors.New("authentication was configured but this component or its host does not support extensions")
348352
}
349353

350-
grpcAuthenticator, cerr := cc.Auth.Get().GetGRPCClientAuthenticator(ctx, host.GetExtensions())
354+
grpcAuthenticator, cerr := cc.Auth.Get().GetGRPCClientAuthenticator(ctx, extensions)
351355
if cerr != nil {
352356
return nil, cerr
353357
}
@@ -388,8 +392,11 @@ func (cc *ClientConfig) getGrpcDialOptions(
388392
}
389393

390394
// Apply middleware options. Note: OpenTelemetry could be registered as an extension.
395+
if len(cc.Middlewares) > 0 && extensions == nil {
396+
return nil, errors.New("middlewares were configured but this component or its host does not support extensions")
397+
}
391398
for _, middleware := range cc.Middlewares {
392-
middlewareOptions, err := middleware.GetGRPCClientOptions(ctx, host.GetExtensions())
399+
middlewareOptions, err := middleware.GetGRPCClientOptions(ctx, extensions)
393400
if err != nil {
394401
return nil, fmt.Errorf("failed to get gRPC client options from middleware: %w", err)
395402
}
@@ -437,13 +444,17 @@ func WithGrpcServerOption(opt grpc.ServerOption) ToServerOption {
437444
func (grpcServerOptionWrapper) isToServerOption() {}
438445

439446
// ToServer returns a [grpc.Server] for the configuration.
447+
//
448+
// To allow the configuration to reference middleware or authentication extensions,
449+
// the `extensions` argument should be the output of `host.GetExtensions()`.
450+
// It may also be `nil` in tests where no such extension is expected to be used.
440451
func (sc *ServerConfig) ToServer(
441452
ctx context.Context,
442-
host component.Host,
453+
extensions map[component.ID]component.Component,
443454
settings component.TelemetrySettings,
444455
extraOpts ...ToServerOption,
445456
) (*grpc.Server, error) {
446-
grpcOpts, err := sc.getGrpcServerOptions(ctx, host, settings, extraOpts)
457+
grpcOpts, err := sc.getGrpcServerOptions(ctx, extensions, settings, extraOpts)
447458
if err != nil {
448459
return nil, err
449460
}
@@ -452,7 +463,7 @@ func (sc *ServerConfig) ToServer(
452463

453464
func (sc *ServerConfig) getGrpcServerOptions(
454465
ctx context.Context,
455-
host component.Host,
466+
extensions map[component.ID]component.Component,
456467
settings component.TelemetrySettings,
457468
extraOpts []ToServerOption,
458469
) ([]grpc.ServerOption, error) {
@@ -515,7 +526,7 @@ func (sc *ServerConfig) getGrpcServerOptions(
515526
var sInterceptors []grpc.StreamServerInterceptor
516527

517528
if sc.Auth.HasValue() {
518-
authenticator, err := sc.Auth.Get().GetServerAuthenticator(ctx, host.GetExtensions())
529+
authenticator, err := sc.Auth.Get().GetServerAuthenticator(ctx, extensions)
519530
if err != nil {
520531
return nil, err
521532
}
@@ -539,7 +550,7 @@ func (sc *ServerConfig) getGrpcServerOptions(
539550

540551
// Apply middleware options. Note: OpenTelemetry could be registered as an extension.
541552
for _, middleware := range sc.Middlewares {
542-
middlewareOptions, err := middleware.GetGRPCServerOptions(ctx, host.GetExtensions())
553+
middlewareOptions, err := middleware.GetGRPCServerOptions(ctx, extensions)
543554
if err != nil {
544555
return nil, fmt.Errorf("failed to get gRPC server options from middleware: %w", err)
545556
}

0 commit comments

Comments
 (0)