diff --git a/.chloggen/mx-psi_extensionauth-refactor-2.yaml b/.chloggen/mx-psi_extensionauth-refactor-2.yaml new file mode 100644 index 00000000000..2babd006159 --- /dev/null +++ b/.chloggen/mx-psi_extensionauth-refactor-2.yaml @@ -0,0 +1,26 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: deprecation + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: extensionauth + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecate `extensionauth.NewClient` and `extensionauth.NewServer`. + +# One or more tracking issues or pull requests related to the change +issues: [12574] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + - Manually implement the interfaces instead. + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/.chloggen/mx-psi_extensionauth-refactor.yaml b/.chloggen/mx-psi_extensionauth-refactor.yaml new file mode 100644 index 00000000000..a5e7313f54e --- /dev/null +++ b/.chloggen/mx-psi_extensionauth-refactor.yaml @@ -0,0 +1,26 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: deprecation + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: configauth + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecate `configauth.Authenticator.GetClientAuthenticator`. + +# One or more tracking issues or pull requests related to the change +issues: [12574] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: | + - Use the per-protocol methods instead. + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/.chloggen/mx-psi_seal-them-all.yaml b/.chloggen/mx-psi_seal-them-all.yaml index 6371a8bae26..727279c1961 100644 --- a/.chloggen/mx-psi_seal-them-all.yaml +++ b/.chloggen/mx-psi_seal-them-all.yaml @@ -16,7 +16,9 @@ issues: [12567] # These lines will be padded with 2 spaces and then inserted directly into the document. # Use pipe (|) for multiline entries. subtext: | - 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. + - Use `extensionauthtest.NewNopClient` to create a client with a noop implementation. + - Use `extensionauthtest.NewErrorClient` to create a client that always returns an error. + - Implement the `extensionauth` interfaces for custom mock client implementations. # Optional: The change log or logs in which this entry should be included. # e.g. '[user]' or '[user, api]' diff --git a/config/configauth/configauth.go b/config/configauth/configauth.go index d2e7f45ff0c..4e2504d171f 100644 --- a/config/configauth/configauth.go +++ b/config/configauth/configauth.go @@ -18,6 +18,8 @@ import ( var ( errAuthenticatorNotFound = errors.New("authenticator not found") errNotClient = errors.New("requested authenticator is not a client authenticator") + errNotHTTPClient = errors.New("requested authenticator is not a HTTP client authenticator") + errNotGRPCClient = errors.New("requested authenticator is not a gRPC client authenticator") errNotServer = errors.New("requested authenticator is not a server authenticator") ) @@ -42,7 +44,7 @@ func (a Authentication) GetServerAuthenticator(_ context.Context, extensions map // GetClientAuthenticator attempts to select the appropriate extensionauth.Client from the list of extensions, // based on the component id of the extension. If an authenticator is not found, an error is returned. -// This should be only used by HTTP clients. +// Deprecated: [v0.122.0] Use GetHTTPClientAuthenticator or GetGRPCClientAuthenticator instead. func (a Authentication) GetClientAuthenticator(_ context.Context, extensions map[component.ID]component.Component) (extensionauth.Client, error) { if ext, found := extensions[a.AuthenticatorID]; found { if client, ok := ext.(extensionauth.Client); ok { @@ -52,3 +54,29 @@ func (a Authentication) GetClientAuthenticator(_ context.Context, extensions map } return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound) } + +// GetHTTPClientAuthenticator attempts to select the appropriate extensionauth.Client from the list of extensions, +// based on the component id of the extension. If an authenticator is not found, an error is returned. +// This should be only used by HTTP clients. +func (a Authentication) GetHTTPClientAuthenticator(_ context.Context, extensions map[component.ID]component.Component) (extensionauth.HTTPClient, error) { + if ext, found := extensions[a.AuthenticatorID]; found { + if client, ok := ext.(extensionauth.HTTPClient); ok { + return client, nil + } + return nil, errNotHTTPClient + } + return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound) +} + +// GetGRPCClientAuthenticator attempts to select the appropriate extensionauth.Client from the list of extensions, +// based on the component id of the extension. If an authenticator is not found, an error is returned. +// This should be only used by gRPC clients. +func (a Authentication) GetGRPCClientAuthenticator(_ context.Context, extensions map[component.ID]component.Component) (extensionauth.GRPCClient, error) { + if ext, found := extensions[a.AuthenticatorID]; found { + if client, ok := ext.(extensionauth.GRPCClient); ok { + return client, nil + } + return nil, errNotGRPCClient + } + return nil, fmt.Errorf("failed to resolve authenticator %q: %w", a.AuthenticatorID, errAuthenticatorNotFound) +} diff --git a/config/configauth/configauth_test.go b/config/configauth/configauth_test.go index f86d7d7abd7..8a7bf23fd5f 100644 --- a/config/configauth/configauth_test.go +++ b/config/configauth/configauth_test.go @@ -12,18 +12,11 @@ import ( "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/extension" - "go.opentelemetry.io/collector/extension/extensionauth" + "go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest" ) var mockID = component.MustNewID("mock") -func must[T any](t *testing.T, builder func() (T, error)) T { - t.Helper() - thing, err := builder() - require.NoError(t, err) - return thing -} - func TestGetServer(t *testing.T) { testCases := []struct { name string @@ -31,18 +24,14 @@ func TestGetServer(t *testing.T) { expected error }{ { - name: "obtain server authenticator", - authenticator: must(t, func() (extension.Extension, error) { - return extensionauth.NewServer() - }), - expected: nil, + name: "obtain server authenticator", + authenticator: extensionauthtest.NewNopServer(), + expected: nil, }, { - name: "not a server authenticator", - authenticator: must(t, func() (extension.Extension, error) { - return extensionauth.NewClient() - }), - expected: errNotServer, + name: "not a server authenticator", + authenticator: extensionauthtest.NewNopClient(), + expected: errNotServer, }, } for _, tt := range testCases { @@ -86,18 +75,14 @@ func TestGetClient(t *testing.T) { expected error }{ { - name: "obtain client authenticator", - authenticator: must(t, func() (extension.Extension, error) { - return extensionauth.NewClient() - }), - expected: nil, + name: "obtain client authenticator", + authenticator: extensionauthtest.NewNopClient(), + expected: nil, }, { - name: "not a client authenticator", - authenticator: must(t, func() (extension.Extension, error) { - return extensionauth.NewServer() - }), - expected: errNotClient, + name: "not a client authenticator", + authenticator: extensionauthtest.NewNopServer(), + expected: errNotHTTPClient, }, } for _, tt := range testCases { @@ -110,7 +95,7 @@ func TestGetClient(t *testing.T) { mockID: tt.authenticator, } - authenticator, err := cfg.GetClientAuthenticator(context.Background(), ext) + authenticator, err := cfg.GetHTTPClientAuthenticator(context.Background(), ext) // verify if tt.expected != nil { @@ -128,7 +113,7 @@ func TestGetClientFails(t *testing.T) { cfg := &Authentication{ AuthenticatorID: component.MustNewID("does_not_exist"), } - authenticator, err := cfg.GetClientAuthenticator(context.Background(), map[component.ID]component.Component{}) + authenticator, err := cfg.GetGRPCClientAuthenticator(context.Background(), map[component.ID]component.Component{}) require.ErrorIs(t, err, errAuthenticatorNotFound) assert.Nil(t, authenticator) } diff --git a/config/configauth/go.mod b/config/configauth/go.mod index 262028098a7..6a386a11763 100644 --- a/config/configauth/go.mod +++ b/config/configauth/go.mod @@ -7,6 +7,7 @@ require ( go.opentelemetry.io/collector/component v1.27.0 go.opentelemetry.io/collector/extension v1.27.0 go.opentelemetry.io/collector/extension/extensionauth v0.121.0 + go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest v0.121.0 go.uber.org/goleak v1.3.0 ) @@ -38,3 +39,5 @@ replace go.opentelemetry.io/collector/component/componenttest => ../../component replace go.opentelemetry.io/collector/extension => ../../extension replace go.opentelemetry.io/collector/extension/extensionauth => ../../extension/extensionauth + +replace go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest => ../../extension/extensionauth/extensionauthtest diff --git a/config/configgrpc/configgrpc.go b/config/configgrpc/configgrpc.go index 2947f2f3820..57890d037c3 100644 --- a/config/configgrpc/configgrpc.go +++ b/config/configgrpc/configgrpc.go @@ -322,7 +322,7 @@ func (gcs *ClientConfig) getGrpcDialOptions( return nil, errors.New("no extensions configuration available") } - grpcAuthenticator, cerr := gcs.Auth.GetClientAuthenticator(ctx, host.GetExtensions()) + grpcAuthenticator, cerr := gcs.Auth.GetGRPCClientAuthenticator(ctx, host.GetExtensions()) if cerr != nil { return nil, cerr } diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index 0584bfac4d1..c0bfceb03f0 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -29,22 +29,25 @@ import ( "go.opentelemetry.io/collector/config/confignet" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/extension/extensionauth" + "go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest" "go.opentelemetry.io/collector/pdata/ptrace/ptraceotlp" ) -func mustNewServerAuth(t *testing.T, opts ...extensionauth.ServerOption) extensionauth.Server { - t.Helper() - srv, err := extensionauth.NewServer(opts...) - require.NoError(t, err) - return srv +var ( + _ extension.Extension = (*mockAuthServer)(nil) + _ extensionauth.Server = (*mockAuthServer)(nil) +) + +type mockAuthServer struct { + component.StartFunc + component.ShutdownFunc + extensionauth.ServerAuthenticateFunc } -func mustNewClientAuth(t *testing.T, opts ...extensionauth.ClientOption) extensionauth.Client { - t.Helper() - client, err := extensionauth.NewClient(opts...) - require.NoError(t, err) - return client +func newMockAuthServer(auth func(ctx context.Context, sources map[string][]string) (context.Context, error)) extensionauth.Server { + return &mockAuthServer{ServerAuthenticateFunc: auth} } func TestNewDefaultKeepaliveClientConfig(t *testing.T) { @@ -177,7 +180,7 @@ func TestAllGrpcClientSettings(t *testing.T) { }, host: &mockHost{ ext: map[component.ID]component.Component{ - testAuthID: mustNewClientAuth(t), + testAuthID: extensionauthtest.NewNopClient(), }, }, }, @@ -206,7 +209,7 @@ func TestAllGrpcClientSettings(t *testing.T) { }, host: &mockHost{ ext: map[component.ID]component.Component{ - testAuthID: mustNewClientAuth(t), + testAuthID: extensionauthtest.NewNopClient(), }, }, }, @@ -235,7 +238,7 @@ func TestAllGrpcClientSettings(t *testing.T) { }, host: &mockHost{ ext: map[component.ID]component.Component{ - testAuthID: mustNewClientAuth(t), + testAuthID: extensionauthtest.NewNopClient(), }, }, }, @@ -415,7 +418,7 @@ func TestGrpcServerAuthSettings(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ - mockID: mustNewServerAuth(t), + mockID: extensionauthtest.NewNopServer(), }, } srv, err := gss.ToServer(context.Background(), host, componenttest.NewNopTelemetrySettings()) @@ -990,7 +993,7 @@ func TestDefaultUnaryInterceptorAuthSucceeded(t *testing.T) { ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs("authorization", "some-auth-data")) // test - res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc))) + res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, newMockAuthServer(authFunc)) // verify assert.Nil(t, res) @@ -1014,7 +1017,7 @@ func TestDefaultUnaryInterceptorAuthFailure(t *testing.T) { ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs("authorization", "some-auth-data")) // test - res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc))) + res, err := authUnaryServerInterceptor(ctx, nil, &grpc.UnaryServerInfo{}, handler, newMockAuthServer(authFunc)) // verify assert.Nil(t, res) @@ -1035,7 +1038,7 @@ func TestDefaultUnaryInterceptorMissingMetadata(t *testing.T) { } // test - res, err := authUnaryServerInterceptor(context.Background(), nil, &grpc.UnaryServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc))) + res, err := authUnaryServerInterceptor(context.Background(), nil, &grpc.UnaryServerInfo{}, handler, newMockAuthServer(authFunc)) // verify assert.Nil(t, res) @@ -1066,7 +1069,7 @@ func TestDefaultStreamInterceptorAuthSucceeded(t *testing.T) { } // test - err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc))) + err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, newMockAuthServer(authFunc)) // verify require.NoError(t, err) @@ -1092,7 +1095,7 @@ func TestDefaultStreamInterceptorAuthFailure(t *testing.T) { } // test - err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc))) + err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, newMockAuthServer(authFunc)) // verify require.ErrorContains(t, err, expectedErr.Error()) // unfortunately, grpc errors don't wrap the original ones @@ -1115,7 +1118,7 @@ func TestDefaultStreamInterceptorMissingMetadata(t *testing.T) { } // test - err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, mustNewServerAuth(t, extensionauth.WithServerAuthenticate(authFunc))) + err := authStreamServerInterceptor(nil, streamServer, &grpc.StreamServerInfo{}, handler, newMockAuthServer(authFunc)) // verify assert.Equal(t, errMetadataNotFound, err) diff --git a/config/configgrpc/go.mod b/config/configgrpc/go.mod index 335e5b7fe90..1caa11e0dc5 100644 --- a/config/configgrpc/go.mod +++ b/config/configgrpc/go.mod @@ -13,7 +13,9 @@ require ( go.opentelemetry.io/collector/config/confignet v1.27.0 go.opentelemetry.io/collector/config/configopaque v1.27.0 go.opentelemetry.io/collector/config/configtls v1.27.0 + go.opentelemetry.io/collector/extension v1.27.0 go.opentelemetry.io/collector/extension/extensionauth v0.121.0 + go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest v0.121.0 go.opentelemetry.io/collector/pdata v1.27.0 go.opentelemetry.io/collector/pdata/testdata v0.121.0 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.60.0 @@ -36,7 +38,6 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect - go.opentelemetry.io/collector/extension v1.27.0 // indirect go.opentelemetry.io/collector/pdata/pprofile v0.121.0 // indirect go.opentelemetry.io/otel/metric v1.35.0 // indirect go.opentelemetry.io/otel/sdk v1.35.0 // indirect @@ -79,3 +80,5 @@ replace go.opentelemetry.io/collector/component => ../../component replace go.opentelemetry.io/collector/component/componenttest => ../../component/componenttest replace go.opentelemetry.io/collector/consumer => ../../consumer + +replace go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest => ../../extension/extensionauth/extensionauthtest diff --git a/config/confighttp/confighttp.go b/config/confighttp/confighttp.go index 40c3ba6de67..06dd9daa560 100644 --- a/config/confighttp/confighttp.go +++ b/config/confighttp/confighttp.go @@ -203,7 +203,7 @@ func (hcs *ClientConfig) ToClient(ctx context.Context, host component.Host, sett return nil, errors.New("extensions configuration not found") } - httpCustomAuthRoundTripper, aerr := hcs.Auth.GetClientAuthenticator(ctx, ext) + httpCustomAuthRoundTripper, aerr := hcs.Auth.GetHTTPClientAuthenticator(ctx, ext) if aerr != nil { return nil, aerr } diff --git a/config/confighttp/confighttp_test.go b/config/confighttp/confighttp_test.go index fa1442aff1e..a60ab07310c 100644 --- a/config/confighttp/confighttp_test.go +++ b/config/confighttp/confighttp_test.go @@ -30,15 +30,24 @@ import ( "go.opentelemetry.io/collector/config/configcompression" "go.opentelemetry.io/collector/config/configopaque" "go.opentelemetry.io/collector/config/configtls" + "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/extension/extensionauth" "go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest" ) -func build[T any, O any](t *testing.T, builder func(...O) (T, error), opts ...O) T { - t.Helper() - obj, err := builder(opts...) - require.NoError(t, err) - return obj +var ( + _ extension.Extension = (*mockAuthServer)(nil) + _ extensionauth.Server = (*mockAuthServer)(nil) +) + +type mockAuthServer struct { + component.StartFunc + component.ShutdownFunc + extensionauth.ServerAuthenticateFunc +} + +func newMockAuthServer(auth func(ctx context.Context, sources map[string][]string) (context.Context, error)) extensionauth.Server { + return &mockAuthServer{ServerAuthenticateFunc: auth} } var ( @@ -53,7 +62,7 @@ var ( func TestAllHTTPClientSettings(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ - testAuthID: build(t, extensionauth.NewClient), + testAuthID: extensionauthtest.NewNopClient(), }, } @@ -179,7 +188,7 @@ func TestAllHTTPClientSettings(t *testing.T) { func TestPartialHTTPClientSettings(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ - testAuthID: build(t, extensionauth.NewClient), + testAuthID: extensionauthtest.NewNopClient(), }, } @@ -339,12 +348,22 @@ func (c *customRoundTripper) RoundTrip(*http.Request) (*http.Response, error) { return nil, nil } -func TestHTTPClientSettingWithAuthConfig(t *testing.T) { - customRoundTripperOpt := extensionauth.WithClientRoundTripper( - func(http.RoundTripper) (http.RoundTripper, error) { - return &customRoundTripper{}, nil - }) +var ( + _ extensionauth.HTTPClient = (*mockClient)(nil) + _ extension.Extension = (*mockClient)(nil) +) + +type mockClient struct { + component.StartFunc + component.ShutdownFunc +} +// RoundTripper implements extensionauth.HTTPClient. +func (m *mockClient) RoundTripper(http.RoundTripper) (http.RoundTripper, error) { + return &customRoundTripper{}, nil +} + +func TestHTTPClientSettingWithAuthConfig(t *testing.T) { tests := []struct { name string shouldErr bool @@ -360,7 +379,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { shouldErr: false, host: &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewClient), + mockID: extensionauthtest.NewNopClient(), }, }, }, @@ -373,7 +392,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { shouldErr: true, host: &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewClient), + mockID: extensionauthtest.NewNopClient(), }, }, }, @@ -395,7 +414,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { shouldErr: false, host: &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewClient, customRoundTripperOpt), + mockID: &mockClient{}, }, }, }, @@ -409,7 +428,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { shouldErr: false, host: &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewClient, customRoundTripperOpt), + mockID: &mockClient{}, }, }, }, @@ -423,7 +442,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { shouldErr: false, host: &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewClient, customRoundTripperOpt), + mockID: &mockClient{}, }, }, }, @@ -436,7 +455,7 @@ func TestHTTPClientSettingWithAuthConfig(t *testing.T) { shouldErr: true, host: &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauthtest.NewErrorClient), + mockID: extensionauthtest.NewErrorClient(errors.New("error")), }, }, }, @@ -833,11 +852,9 @@ func TestHttpCorsWithSettings(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewServer, - extensionauth.WithServerAuthenticate(func(ctx context.Context, _ map[string][]string) (context.Context, error) { - return ctx, errors.New("Settings failed") - }), - ), + mockID: newMockAuthServer(func(ctx context.Context, _ map[string][]string) (context.Context, error) { + return ctx, errors.New("Settings failed") + }), }, } @@ -1145,12 +1162,10 @@ func TestServerAuth(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewServer, - extensionauth.WithServerAuthenticate(func(ctx context.Context, _ map[string][]string) (context.Context, error) { - authCalled = true - return ctx, nil - }), - ), + mockID: newMockAuthServer(func(ctx context.Context, _ map[string][]string) (context.Context, error) { + authCalled = true + return ctx, nil + }), }, } @@ -1196,11 +1211,9 @@ func TestFailedServerAuth(t *testing.T) { } host := &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewServer, - extensionauth.WithServerAuthenticate(func(ctx context.Context, _ map[string][]string) (context.Context, error) { - return ctx, errors.New("Settings failed") - }), - ), + mockID: newMockAuthServer(func(ctx context.Context, _ map[string][]string) (context.Context, error) { + return ctx, errors.New("Settings failed") + }), }, } @@ -1377,14 +1390,12 @@ func TestAuthWithQueryParams(t *testing.T) { host := &mockHost{ ext: map[component.ID]component.Component{ - mockID: build(t, extensionauth.NewServer, - extensionauth.WithServerAuthenticate(func(ctx context.Context, sources map[string][]string) (context.Context, error) { - require.Len(t, sources, 1) - assert.Equal(t, "1", sources["auth"][0]) - authCalled = true - return ctx, nil - }), - ), + mockID: newMockAuthServer(func(ctx context.Context, sources map[string][]string) (context.Context, error) { + require.Len(t, sources, 1) + assert.Equal(t, "1", sources["auth"][0]) + authCalled = true + return ctx, nil + }), }, } diff --git a/config/confighttp/go.mod b/config/confighttp/go.mod index 67e5a602151..b20d6a0b33f 100644 --- a/config/confighttp/go.mod +++ b/config/confighttp/go.mod @@ -15,6 +15,7 @@ require ( go.opentelemetry.io/collector/config/configcompression v1.27.0 go.opentelemetry.io/collector/config/configopaque v1.27.0 go.opentelemetry.io/collector/config/configtls v1.27.0 + go.opentelemetry.io/collector/extension v1.27.0 go.opentelemetry.io/collector/extension/extensionauth v0.121.0 go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest v0.121.0 go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.60.0 @@ -34,7 +35,6 @@ require ( github.com/google/uuid v1.6.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect go.opentelemetry.io/auto/sdk v1.1.0 // indirect - go.opentelemetry.io/collector/extension v1.27.0 // indirect go.opentelemetry.io/collector/pdata v1.27.0 // indirect go.opentelemetry.io/otel/metric v1.35.0 // indirect go.opentelemetry.io/otel/sdk v1.35.0 // indirect diff --git a/exporter/otlpexporter/go.mod b/exporter/otlpexporter/go.mod index 855bcee50c7..3012addc4b1 100644 --- a/exporter/otlpexporter/go.mod +++ b/exporter/otlpexporter/go.mod @@ -155,3 +155,5 @@ replace go.opentelemetry.io/collector/extension/extensiontest => ../../extension replace go.opentelemetry.io/collector/featuregate => ../../featuregate replace go.opentelemetry.io/collector/extension/xextension => ../../extension/xextension + +replace go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest => ../../extension/extensionauth/extensionauthtest diff --git a/extension/extensionauth/client.go b/extension/extensionauth/client.go index d05a07a6818..0d00a58c118 100644 --- a/extension/extensionauth/client.go +++ b/extension/extensionauth/client.go @@ -12,20 +12,34 @@ import ( "go.opentelemetry.io/collector/extension" ) -// Client is an Extension that can be used as an authenticator for the configauth.Authentication option. +// Client is an optional Extension interface that can be used as an HTTP and gRPC authenticator for the configauth.Authentication option. // Authenticators are then included as part of OpenTelemetry Collector builds and can be referenced by their // names from the Authentication configuration. +// Deprecated: [v0.122.0] Assert the use of HTTPClient and GRPCClient interfaces instead. type Client interface { extension.Extension + HTTPClient + GRPCClient +} +// HTTPClient is an optional Extension interface that can be used as an HTTP authenticator for the configauth.Authentication option. +// Authenticators are then included as part of OpenTelemetry Collector builds and can be referenced by their +// names from the Authentication configuration. +type HTTPClient interface { // RoundTripper returns a RoundTripper that can be used to authenticate HTTP requests. RoundTripper(base http.RoundTripper) (http.RoundTripper, error) +} +// GRPCClient is an optional Extension interface that can be used as a gRPC authenticator for the configauth.Authentication option. +// Authenticators are then included as part of OpenTelemetry Collector builds and can be referenced by their +// names from the Authentication configuration. +type GRPCClient interface { // PerRPCCredentials returns a PerRPCCredentials that can be used to authenticate gRPC requests. PerRPCCredentials() (credentials.PerRPCCredentials, error) } // ClientOption represents the possible options for NewClient. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. type ClientOption interface { apply(*defaultClient) } @@ -36,12 +50,30 @@ func (of clientOptionFunc) apply(e *defaultClient) { of(e) } +var _ HTTPClient = (*ClientRoundTripperFunc)(nil) + // ClientRoundTripperFunc specifies the function that returns a RoundTripper that can be used to authenticate HTTP requests. type ClientRoundTripperFunc func(base http.RoundTripper) (http.RoundTripper, error) +func (f ClientRoundTripperFunc) RoundTripper(base http.RoundTripper) (http.RoundTripper, error) { + if f == nil { + return base, nil + } + return f(base) +} + +var _ GRPCClient = (*ClientPerRPCCredentialsFunc)(nil) + // ClientPerRPCCredentialsFunc specifies the function that returns a PerRPCCredentials that can be used to authenticate gRPC requests. type ClientPerRPCCredentialsFunc func() (credentials.PerRPCCredentials, error) +func (f ClientPerRPCCredentialsFunc) PerRPCCredentials() (credentials.PerRPCCredentials, error) { + if f == nil { + return nil, nil + } + return f() +} + var _ Client = (*defaultClient)(nil) type defaultClient struct { @@ -69,6 +101,7 @@ func (d *defaultClient) RoundTripper(base http.RoundTripper) (http.RoundTripper, // WithClientStart overrides the default `Start` function for a component.Component. // The default always returns nil. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithClientStart(startFunc component.StartFunc) ClientOption { return clientOptionFunc(func(o *defaultClient) { o.StartFunc = startFunc @@ -77,6 +110,7 @@ func WithClientStart(startFunc component.StartFunc) ClientOption { // WithClientShutdown overrides the default `Shutdown` function for a component.Component. // The default always returns nil. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithClientShutdown(shutdownFunc component.ShutdownFunc) ClientOption { return clientOptionFunc(func(o *defaultClient) { o.ShutdownFunc = shutdownFunc @@ -85,6 +119,7 @@ func WithClientShutdown(shutdownFunc component.ShutdownFunc) ClientOption { // WithClientRoundTripper provides a `RoundTripper` function for this client authenticator. // The default round tripper is no-op. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithClientRoundTripper(roundTripperFunc ClientRoundTripperFunc) ClientOption { return clientOptionFunc(func(o *defaultClient) { o.clientRoundTripperFunc = roundTripperFunc @@ -93,6 +128,7 @@ func WithClientRoundTripper(roundTripperFunc ClientRoundTripperFunc) ClientOptio // WithClientPerRPCCredentials provides a `PerRPCCredentials` function for this client authenticator. // There's no default. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithClientPerRPCCredentials(perRPCCredentialsFunc ClientPerRPCCredentialsFunc) ClientOption { return clientOptionFunc(func(o *defaultClient) { o.clientPerRPCCredentialsFunc = perRPCCredentialsFunc @@ -100,6 +136,8 @@ func WithClientPerRPCCredentials(perRPCCredentialsFunc ClientPerRPCCredentialsFu } // NewClient returns a Client configured with the provided options. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. +// Manually implement the [HTTPClient] and/or [GRPCClient] interfaces instead. func NewClient(options ...ClientOption) (Client, error) { bc := &defaultClient{} diff --git a/extension/extensionauth/extensionauthtest/error_client.go b/extension/extensionauth/extensionauthtest/error_client.go index 6237453f162..7ccbafc1220 100644 --- a/extension/extensionauth/extensionauthtest/error_client.go +++ b/extension/extensionauth/extensionauthtest/error_client.go @@ -4,25 +4,36 @@ package extensionauthtest // import "go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest" import ( - "errors" "net/http" "google.golang.org/grpc/credentials" + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" "go.opentelemetry.io/collector/extension/extensionauth" ) -var errMockError = errors.New("mock Error") +var ( + _ extension.Extension = (*errorClient)(nil) + _ extensionauth.HTTPClient = (*errorClient)(nil) + _ extensionauth.GRPCClient = (*errorClient)(nil) +) + +type errorClient struct { + component.StartFunc + component.ShutdownFunc + extensionauth.ClientPerRPCCredentialsFunc + extensionauth.ClientRoundTripperFunc +} -// NewErrorClient returns a new extensionauth.Client that always returns an error on any of the client methods. -func NewErrorClient(opts ...extensionauth.ClientOption) (extensionauth.Client, error) { - errorOpts := []extensionauth.ClientOption{ - extensionauth.WithClientRoundTripper(func(http.RoundTripper) (http.RoundTripper, error) { - return nil, errMockError - }), - extensionauth.WithClientPerRPCCredentials(func() (credentials.PerRPCCredentials, error) { - return nil, errMockError - }), +// NewErrorClient returns a new [extension.Extension] that implements the [extensionauth.HTTPClient] and [extensionauth.GRPCClient] and always returns an error on both methods. +func NewErrorClient(err error) extension.Extension { + return &errorClient{ + ClientRoundTripperFunc: func(http.RoundTripper) (http.RoundTripper, error) { + return nil, err + }, + ClientPerRPCCredentialsFunc: func() (credentials.PerRPCCredentials, error) { + return nil, err + }, } - return extensionauth.NewClient(append(errorOpts, opts...)...) } diff --git a/extension/extensionauth/extensionauthtest/error_client_test.go b/extension/extensionauth/extensionauthtest/error_client_test.go index a688d08e5f1..fd34fa96275 100644 --- a/extension/extensionauth/extensionauthtest/error_client_test.go +++ b/extension/extensionauth/extensionauthtest/error_client_test.go @@ -4,18 +4,24 @@ package extensionauthtest import ( + "errors" "testing" "github.com/stretchr/testify/require" + + "go.opentelemetry.io/collector/extension/extensionauth" ) func TestErrorClient(t *testing.T) { - client, err := NewErrorClient() - require.NoError(t, err) + client := NewErrorClient(errors.New("error")) - _, err = client.RoundTripper(nil) + httpClient, ok := client.(extensionauth.HTTPClient) + require.True(t, ok) + _, err := httpClient.RoundTripper(nil) require.Error(t, err) - _, err = client.PerRPCCredentials() + grpcClient, ok := client.(extensionauth.GRPCClient) + require.True(t, ok) + _, err = grpcClient.PerRPCCredentials() require.Error(t, err) } diff --git a/extension/extensionauth/extensionauthtest/go.mod b/extension/extensionauth/extensionauthtest/go.mod index 94b483a51b3..b3aee16f176 100644 --- a/extension/extensionauth/extensionauthtest/go.mod +++ b/extension/extensionauth/extensionauthtest/go.mod @@ -4,6 +4,8 @@ go 1.23.0 require ( github.com/stretchr/testify v1.10.0 + go.opentelemetry.io/collector/component v1.27.0 + go.opentelemetry.io/collector/extension v1.27.0 go.opentelemetry.io/collector/extension/extensionauth v0.121.0 go.uber.org/goleak v1.3.0 google.golang.org/grpc v1.71.0 @@ -13,8 +15,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - go.opentelemetry.io/collector/component v1.27.0 // indirect - go.opentelemetry.io/collector/extension v1.27.0 // indirect go.opentelemetry.io/collector/pdata v1.27.0 // indirect go.opentelemetry.io/otel v1.35.0 // indirect go.opentelemetry.io/otel/metric v1.35.0 // indirect diff --git a/extension/extensionauth/extensionauthtest/nop_client.go b/extension/extensionauth/extensionauthtest/nop_client.go new file mode 100644 index 00000000000..077889c3a95 --- /dev/null +++ b/extension/extensionauth/extensionauthtest/nop_client.go @@ -0,0 +1,29 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package extensionauthtest // import "go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest" + +import ( + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" + "go.opentelemetry.io/collector/extension/extensionauth" +) + +var ( + _ extension.Extension = (*nopClient)(nil) + _ extensionauth.HTTPClient = (*nopClient)(nil) + _ extensionauth.GRPCClient = (*nopClient)(nil) +) + +type nopClient struct { + component.StartFunc + component.ShutdownFunc + extensionauth.ClientRoundTripperFunc + extensionauth.ClientPerRPCCredentialsFunc +} + +// NewNopClient returns a new [extension.Extension] that implements the [extensionauth.HTTPClient] and [extensionauth.GRPCClient]. +// For HTTP requests it returns the base RoundTripper and for gRPC requests it returns a nil [credentials.PerRPCCredentials]. +func NewNopClient() extension.Extension { + return &nopClient{} +} diff --git a/extension/extensionauth/extensionauthtest/nop_client_test.go b/extension/extensionauth/extensionauthtest/nop_client_test.go new file mode 100644 index 00000000000..5a202957ad8 --- /dev/null +++ b/extension/extensionauth/extensionauthtest/nop_client_test.go @@ -0,0 +1,29 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package extensionauthtest + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "go.opentelemetry.io/collector/extension/extensionauth" +) + +func TestNopClient(t *testing.T) { + client := NewNopClient() + + httpClient, ok := client.(extensionauth.HTTPClient) + require.True(t, ok) + rt, err := httpClient.RoundTripper(nil) + require.NoError(t, err) + assert.Nil(t, rt) + + grpcClient, ok := client.(extensionauth.GRPCClient) + require.True(t, ok) + grpcAuth, err := grpcClient.PerRPCCredentials() + require.NoError(t, err) + assert.Nil(t, grpcAuth) +} diff --git a/extension/extensionauth/extensionauthtest/nop_server.go b/extension/extensionauth/extensionauthtest/nop_server.go new file mode 100644 index 00000000000..de548603950 --- /dev/null +++ b/extension/extensionauth/extensionauthtest/nop_server.go @@ -0,0 +1,32 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package extensionauthtest // import "go.opentelemetry.io/collector/extension/extensionauth/extensionauthtest" + +import ( + "context" + + "go.opentelemetry.io/collector/component" + "go.opentelemetry.io/collector/extension" + "go.opentelemetry.io/collector/extension/extensionauth" +) + +var ( + _ extension.Extension = (*nopServer)(nil) + _ extensionauth.Server = (*nopServer)(nil) +) + +type nopServer struct { + component.StartFunc + component.ShutdownFunc +} + +// Authenticate implements extensionauth.Server. +func (n *nopServer) Authenticate(ctx context.Context, _ map[string][]string) (context.Context, error) { + return ctx, nil +} + +// NewNopServer returns a new extension.Extension that implements the extensionauth.Server. +func NewNopServer() extension.Extension { + return &nopServer{} +} diff --git a/extension/extensionauth/server.go b/extension/extensionauth/server.go index c29d1b0b466..eac3b1feca3 100644 --- a/extension/extensionauth/server.go +++ b/extension/extensionauth/server.go @@ -10,12 +10,13 @@ import ( "go.opentelemetry.io/collector/extension" ) -// Server is an Extension that can be used as an authenticator for the configauth.Authentication option. +// Server is an optional Extension interface that can be used as an authenticator for the configauth.Authentication option. // Authenticators are then included as part of OpenTelemetry Collector builds and can be referenced by their // names from the Authentication configuration. Each Server is free to define its own behavior and configuration options, // but note that the expectations that come as part of Extensions exist here as well. For instance, multiple instances of the same // authenticator should be possible to exist under different names. type Server interface { + // Deprecated: will be removed in the next release. extension.Extension // Authenticate checks whether the given map contains valid auth data. Successfully authenticated calls will always return a nil error. @@ -60,7 +61,6 @@ func (of serverOptionFunc) apply(e *defaultServer) { // on the given sources map. See Server.Authenticate. type ServerAuthenticateFunc func(ctx context.Context, sources map[string][]string) (context.Context, error) -// Deprecated: [v0.121.0] No longer used, will be removed. func (f ServerAuthenticateFunc) Authenticate(ctx context.Context, sources map[string][]string) (context.Context, error) { if f == nil { return ctx, nil @@ -69,6 +69,7 @@ func (f ServerAuthenticateFunc) Authenticate(ctx context.Context, sources map[st } // WithServerAuthenticate specifies which function to use to perform the authentication. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithServerAuthenticate(authFunc ServerAuthenticateFunc) ServerOption { return serverOptionFunc(func(o *defaultServer) { o.serverAuthenticateFunc = authFunc @@ -77,6 +78,7 @@ func WithServerAuthenticate(authFunc ServerAuthenticateFunc) ServerOption { // WithServerStart overrides the default `Start` function for a component.Component. // The default always returns nil. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithServerStart(startFunc component.StartFunc) ServerOption { return serverOptionFunc(func(o *defaultServer) { o.StartFunc = startFunc @@ -85,6 +87,7 @@ func WithServerStart(startFunc component.StartFunc) ServerOption { // WithServerShutdown overrides the default `Shutdown` function for a component.Component. // The default always returns nil. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. func WithServerShutdown(shutdownFunc component.ShutdownFunc) ServerOption { return serverOptionFunc(func(o *defaultServer) { o.ShutdownFunc = shutdownFunc @@ -92,6 +95,8 @@ func WithServerShutdown(shutdownFunc component.ShutdownFunc) ServerOption { } // NewServer returns a Server configured with the provided options. +// Deprecated: [v0.122.0] This type is deprecated and will be removed in the next release. +// Manually implement the [Server] interface instead. func NewServer(options ...ServerOption) (Server, error) { bc := &defaultServer{}