Skip to content

Commit 1012122

Browse files
[extensionauth] Split extensionauth.Client by protocol type (#12574)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description An attempt at splitting `extensionauth.Client` by protocol type. open-telemetry/opentelemetry-collector-contrib/pull/38451 removes usages of the `NewClient` and `NewServer` constructor --------- Co-authored-by: Jade Guiton <[email protected]>
1 parent 83db91f commit 1012122

21 files changed

+358
-119
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: extensionauth
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Deprecate `extensionauth.NewClient` and `extensionauth.NewServer`.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12574]
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+
- Manually implement the interfaces instead.
20+
21+
# Optional: The change log or logs in which this entry should be included.
22+
# e.g. '[user]' or '[user, api]'
23+
# Include 'user' if the change is relevant to end users.
24+
# Include 'api' if there is a change to a library API.
25+
# Default: '[user]'
26+
change_logs: [api]
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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: deprecation
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: configauth
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Deprecate `configauth.Authenticator.GetClientAuthenticator`.
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12574]
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+
- Use the per-protocol methods instead.
20+
21+
# Optional: The change log or logs in which this entry should be included.
22+
# e.g. '[user]' or '[user, api]'
23+
# Include 'user' if the change is relevant to end users.
24+
# Include 'api' if there is a change to a library API.
25+
# Default: '[user]'
26+
change_logs: [api]

.chloggen/mx-psi_seal-them-all.yaml

+3-1
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@ issues: [12567]
1616
# These lines will be padded with 2 spaces and then inserted directly into the document.
1717
# Use pipe (|) for multiline entries.
1818
subtext: |
19-
Use the standard `extensionauth.NewClient` constructor to create a client with a specific mock implementation. Use `extensionauthtest.NewErrorClient` to create a client that always returns an error.
19+
- Use `extensionauthtest.NewNopClient` to create a client with a noop implementation.
20+
- Use `extensionauthtest.NewErrorClient` to create a client that always returns an error.
21+
- Implement the `extensionauth` interfaces for custom mock client implementations.
2022
2123
# Optional: The change log or logs in which this entry should be included.
2224
# e.g. '[user]' or '[user, api]'

config/configauth/configauth.go

+29-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import (
1818
var (
1919
errAuthenticatorNotFound = errors.New("authenticator not found")
2020
errNotClient = errors.New("requested authenticator is not a client authenticator")
21+
errNotHTTPClient = errors.New("requested authenticator is not a HTTP client authenticator")
22+
errNotGRPCClient = errors.New("requested authenticator is not a gRPC client authenticator")
2123
errNotServer = errors.New("requested authenticator is not a server authenticator")
2224
)
2325

@@ -42,7 +44,7 @@ func (a Authentication) GetServerAuthenticator(_ context.Context, extensions map
4244

4345
// GetClientAuthenticator attempts to select the appropriate extensionauth.Client from the list of extensions,
4446
// based on the component id of the extension. If an authenticator is not found, an error is returned.
45-
// This should be only used by HTTP clients.
47+
// Deprecated: [v0.122.0] Use GetHTTPClientAuthenticator or GetGRPCClientAuthenticator instead.
4648
func (a Authentication) GetClientAuthenticator(_ context.Context, extensions map[component.ID]component.Component) (extensionauth.Client, error) {
4749
if ext, found := extensions[a.AuthenticatorID]; found {
4850
if client, ok := ext.(extensionauth.Client); ok {
@@ -52,3 +54,29 @@ func (a Authentication) GetClientAuthenticator(_ context.Context, extensions map
5254
}
5355
return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound)
5456
}
57+
58+
// GetHTTPClientAuthenticator attempts to select the appropriate extensionauth.Client from the list of extensions,
59+
// based on the component id of the extension. If an authenticator is not found, an error is returned.
60+
// This should be only used by HTTP clients.
61+
func (a Authentication) GetHTTPClientAuthenticator(_ context.Context, extensions map[component.ID]component.Component) (extensionauth.HTTPClient, error) {
62+
if ext, found := extensions[a.AuthenticatorID]; found {
63+
if client, ok := ext.(extensionauth.HTTPClient); ok {
64+
return client, nil
65+
}
66+
return nil, errNotHTTPClient
67+
}
68+
return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound)
69+
}
70+
71+
// GetGRPCClientAuthenticator attempts to select the appropriate extensionauth.Client from the list of extensions,
72+
// based on the component id of the extension. If an authenticator is not found, an error is returned.
73+
// This should be only used by gRPC clients.
74+
func (a Authentication) GetGRPCClientAuthenticator(_ context.Context, extensions map[component.ID]component.Component) (extensionauth.GRPCClient, error) {
75+
if ext, found := extensions[a.AuthenticatorID]; found {
76+
if client, ok := ext.(extensionauth.GRPCClient); ok {
77+
return client, nil
78+
}
79+
return nil, errNotGRPCClient
80+
}
81+
return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound)
82+
}

config/configauth/configauth_test.go

+15-30
Original file line numberDiff line numberDiff line change
@@ -12,37 +12,26 @@ import (
1212

1313
"go.opentelemetry.io/collector/component"
1414
"go.opentelemetry.io/collector/extension"
15-
"go.opentelemetry.io/collector/extension/extensionauth"
15+
"go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest"
1616
)
1717

1818
var mockID = component.MustNewID("mock")
1919

20-
func must[T any](t *testing.T, builder func() (T, error)) T {
21-
t.Helper()
22-
thing, err := builder()
23-
require.NoError(t, err)
24-
return thing
25-
}
26-
2720
func TestGetServer(t *testing.T) {
2821
testCases := []struct {
2922
name string
3023
authenticator extension.Extension
3124
expected error
3225
}{
3326
{
34-
name: "obtain server authenticator",
35-
authenticator: must(t, func() (extension.Extension, error) {
36-
return extensionauth.NewServer()
37-
}),
38-
expected: nil,
27+
name: "obtain server authenticator",
28+
authenticator: extensionauthtest.NewNopServer(),
29+
expected: nil,
3930
},
4031
{
41-
name: "not a server authenticator",
42-
authenticator: must(t, func() (extension.Extension, error) {
43-
return extensionauth.NewClient()
44-
}),
45-
expected: errNotServer,
32+
name: "not a server authenticator",
33+
authenticator: extensionauthtest.NewNopClient(),
34+
expected: errNotServer,
4635
},
4736
}
4837
for _, tt := range testCases {
@@ -86,18 +75,14 @@ func TestGetClient(t *testing.T) {
8675
expected error
8776
}{
8877
{
89-
name: "obtain client authenticator",
90-
authenticator: must(t, func() (extension.Extension, error) {
91-
return extensionauth.NewClient()
92-
}),
93-
expected: nil,
78+
name: "obtain client authenticator",
79+
authenticator: extensionauthtest.NewNopClient(),
80+
expected: nil,
9481
},
9582
{
96-
name: "not a client authenticator",
97-
authenticator: must(t, func() (extension.Extension, error) {
98-
return extensionauth.NewServer()
99-
}),
100-
expected: errNotClient,
83+
name: "not a client authenticator",
84+
authenticator: extensionauthtest.NewNopServer(),
85+
expected: errNotHTTPClient,
10186
},
10287
}
10388
for _, tt := range testCases {
@@ -110,7 +95,7 @@ func TestGetClient(t *testing.T) {
11095
mockID: tt.authenticator,
11196
}
11297

113-
authenticator, err := cfg.GetClientAuthenticator(context.Background(), ext)
98+
authenticator, err := cfg.GetHTTPClientAuthenticator(context.Background(), ext)
11499

115100
// verify
116101
if tt.expected != nil {
@@ -128,7 +113,7 @@ func TestGetClientFails(t *testing.T) {
128113
cfg := &Authentication{
129114
AuthenticatorID: component.MustNewID("does_not_exist"),
130115
}
131-
authenticator, err := cfg.GetClientAuthenticator(context.Background(), map[component.ID]component.Component{})
116+
authenticator, err := cfg.GetGRPCClientAuthenticator(context.Background(), map[component.ID]component.Component{})
132117
require.ErrorIs(t, err, errAuthenticatorNotFound)
133118
assert.Nil(t, authenticator)
134119
}

config/configauth/go.mod

+3
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ require (
77
go.opentelemetry.io/collector/component v1.27.0
88
go.opentelemetry.io/collector/extension v1.27.0
99
go.opentelemetry.io/collector/extension/extensionauth v0.121.0
10+
go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest v0.121.0
1011
go.uber.org/goleak v1.3.0
1112
)
1213

@@ -38,3 +39,5 @@ replace go.opentelemetry.io/collector/component/componenttest => ../../component
3839
replace go.opentelemetry.io/collector/extension => ../../extension
3940

4041
replace go.opentelemetry.io/collector/extension/extensionauth => ../../extension/extensionauth
42+
43+
replace go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest => ../../extension/extensionauth/extensionauthtest

config/configgrpc/configgrpc.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ func (gcs *ClientConfig) getGrpcDialOptions(
322322
return nil, errors.New("no extensions configuration available")
323323
}
324324

325-
grpcAuthenticator, cerr := gcs.Auth.GetClientAuthenticator(ctx, host.GetExtensions())
325+
grpcAuthenticator, cerr := gcs.Auth.GetGRPCClientAuthenticator(ctx, host.GetExtensions())
326326
if cerr != nil {
327327
return nil, cerr
328328
}

config/configgrpc/configgrpc_test.go

+23-20
Original file line numberDiff line numberDiff line change
@@ -29,22 +29,25 @@ import (
2929
"go.opentelemetry.io/collector/config/confignet"
3030
"go.opentelemetry.io/collector/config/configopaque"
3131
"go.opentelemetry.io/collector/config/configtls"
32+
"go.opentelemetry.io/collector/extension"
3233
"go.opentelemetry.io/collector/extension/extensionauth"
34+
"go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest"
3335
"go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp"
3436
)
3537

36-
func mustNewServerAuth(t *testing.T, opts ...extensionauth.ServerOption) extensionauth.Server {
37-
t.Helper()
38-
srv, err := extensionauth.NewServer(opts...)
39-
require.NoError(t, err)
40-
return srv
38+
var (
39+
_ extension.Extension = (*mockAuthServer)(nil)
40+
_ extensionauth.Server = (*mockAuthServer)(nil)
41+
)
42+
43+
type mockAuthServer struct {
44+
component.StartFunc
45+
component.ShutdownFunc
46+
extensionauth.ServerAuthenticateFunc
4147
}
4248

43-
func mustNewClientAuth(t *testing.T, opts ...extensionauth.ClientOption) extensionauth.Client {
44-
t.Helper()
45-
client, err := extensionauth.NewClient(opts...)
46-
require.NoError(t, err)
47-
return client
49+
func newMockAuthServer(auth func(ctx context.Context, sources map[string][]string) (context.Context, error)) extensionauth.Server {
50+
return &mockAuthServer{ServerAuthenticateFunc: auth}
4851
}
4952

5053
func TestNewDefaultKeepaliveClientConfig(t *testing.T) {
@@ -177,7 +180,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
177180
},
178181
host: &mockHost{
179182
ext: map[component.ID]component.Component{
180-
testAuthID: mustNewClientAuth(t),
183+
testAuthID: extensionauthtest.NewNopClient(),
181184
},
182185
},
183186
},
@@ -206,7 +209,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
206209
},
207210
host: &mockHost{
208211
ext: map[component.ID]component.Component{
209-
testAuthID: mustNewClientAuth(t),
212+
testAuthID: extensionauthtest.NewNopClient(),
210213
},
211214
},
212215
},
@@ -235,7 +238,7 @@ func TestAllGrpcClientSettings(t *testing.T) {
235238
},
236239
host: &mockHost{
237240
ext: map[component.ID]component.Component{
238-
testAuthID: mustNewClientAuth(t),
241+
testAuthID: extensionauthtest.NewNopClient(),
239242
},
240243
},
241244
},
@@ -415,7 +418,7 @@ func TestGrpcServerAuthSettings(t *testing.T) {
415418

416419
host := &mockHost{
417420
ext: map[component.ID]component.Component{
418-
mockID: mustNewServerAuth(t),
421+
mockID: extensionauthtest.NewNopServer(),
419422
},
420423
}
421424
srv, err := gss.ToServer(context.Background(), host, componenttest.NewNopTelemetrySettings())
@@ -990,7 +993,7 @@ func TestDefaultUnaryInterceptorAuthSucceeded(t *testing.T) {
990993
ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs("authorization", "some-auth-data"))
991994

992995
// test
993-
res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc)))
996+
res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, newMockAuthServer(authFunc))
994997

995998
// verify
996999
assert.Nil(t, res)
@@ -1014,7 +1017,7 @@ func TestDefaultUnaryInterceptorAuthFailure(t *testing.T) {
10141017
ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs("authorization", "some-auth-data"))
10151018

10161019
// test
1017-
res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc)))
1020+
res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, newMockAuthServer(authFunc))
10181021

10191022
// verify
10201023
assert.Nil(t, res)
@@ -1035,7 +1038,7 @@ func TestDefaultUnaryInterceptorMissingMetadata(t *testing.T) {
10351038
}
10361039

10371040
// test
1038-
res, err := authUnaryServerInterceptor(context.Background(), nil, &grpc.UnaryServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc)))
1041+
res, err := authUnaryServerInterceptor(context.Background(), nil, &grpc.UnaryServerInfo{}, handler, newMockAuthServer(authFunc))
10391042

10401043
// verify
10411044
assert.Nil(t, res)
@@ -1066,7 +1069,7 @@ func TestDefaultStreamInterceptorAuthSucceeded(t *testing.T) {
10661069
}
10671070

10681071
// test
1069-
err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc)))
1072+
err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, newMockAuthServer(authFunc))
10701073

10711074
// verify
10721075
require.NoError(t, err)
@@ -1092,7 +1095,7 @@ func TestDefaultStreamInterceptorAuthFailure(t *testing.T) {
10921095
}
10931096

10941097
// test
1095-
err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc)))
1098+
err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, newMockAuthServer(authFunc))
10961099

10971100
// verify
10981101
require.ErrorContains(t, err, expectedErr.Error()) // unfortunately, grpc errors don't wrap the original ones
@@ -1115,7 +1118,7 @@ func TestDefaultStreamInterceptorMissingMetadata(t *testing.T) {
11151118
}
11161119

11171120
// test
1118-
err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc)))
1121+
err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, newMockAuthServer(authFunc))
11191122

11201123
// verify
11211124
assert.Equal(t, errMetadataNotFound, err)

config/configgrpc/go.mod

+4-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@ require (
1313
go.opentelemetry.io/collector/config/confignet v1.27.0
1414
go.opentelemetry.io/collector/config/configopaque v1.27.0
1515
go.opentelemetry.io/collector/config/configtls v1.27.0
16+
go.opentelemetry.io/collector/extension v1.27.0
1617
go.opentelemetry.io/collector/extension/extensionauth v0.121.0
18+
go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest v0.121.0
1719
go.opentelemetry.io/collector/pdata v1.27.0
1820
go.opentelemetry.io/collector/pdata/testdata v0.121.0
1921
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.60.0
@@ -36,7 +38,6 @@ require (
3638
github.com/modern-go/reflect2 v1.0.2 // indirect
3739
github.com/pmezard/go-difflib v1.0.0 // indirect
3840
go.opentelemetry.io/auto/sdk v1.1.0 // indirect
39-
go.opentelemetry.io/collector/extension v1.27.0 // indirect
4041
go.opentelemetry.io/collector/pdata/pprofile v0.121.0 // indirect
4142
go.opentelemetry.io/otel/metric v1.35.0 // indirect
4243
go.opentelemetry.io/otel/sdk v1.35.0 // indirect
@@ -79,3 +80,5 @@ replace go.opentelemetry.io/collector/component => ../../component
7980
replace go.opentelemetry.io/collector/component/componenttest => ../../component/componenttest
8081

8182
replace go.opentelemetry.io/collector/consumer => ../../consumer
83+
84+
replace go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest => ../../extension/extensionauth/extensionauthtest

config/confighttp/confighttp.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ func (hcs *ClientConfig) ToClient(ctx context.Context, host component.Host, sett
203203
return nil, errors.New("extensions configuration not found")
204204
}
205205

206-
httpCustomAuthRoundTripper, aerr := hcs.Auth.GetClientAuthenticator(ctx, ext)
206+
httpCustomAuthRoundTripper, aerr := hcs.Auth.GetHTTPClientAuthenticator(ctx, ext)
207207
if aerr != nil {
208208
return nil, aerr
209209
}

0 commit comments

Comments
 (0)