Remove interceptors from configauth interface (open-telemetry#4583)

* Remove interceptors from configauth interface Fixes open-telemetry#4582 Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Add changelog entry Signed-off-by: Juraci Paixão Kröhling <[email protected]> * Update CHANGELOG.md Co-authored-by: Bogdan Drutu <[email protected]>
mantian2020 · Dec 20, 2021 · e6f0e01 · e6f0e01
1 parent 6c35df2
commit e6f0e01
Show file tree

Hide file tree

Showing 9 changed files with 216 additions and 320 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,13 +6,13 @@
 
 - Remove `configmapprovider.NewInMemory()` (#4507)
 - Disallow direct implementation of `configmapprovider.Retrieved` (#4577)
+- `configauth`: remove interceptor functions from the ServerAuthenticator interface (#4583)
 
 ## 💡 Enhancements 💡
 
 - `confighttp`: add client-side compression support. (#4441)
   - Each exporter should remove `compression` field if they have and should use `confighttp.HTTPClientSettings`
 - Allow more zap logger configs: `disable_caller`, `disable_stacktrace`, `output_paths`, `error_output_paths`, `initial_fields` (#1048)
-- `configauth`: add ServerAuthenticator interfaces for HTTP receivers. (#4506)
 - Collector self-metrics may now be configured through the configuration file. (#4069)
   - CLI flags for configuring self-metrics are deprecated and will be removed
     in a future release.

diff --git a/config/configauth/mock_serverauth.go b/config/configauth/mock_serverauth.go
@@ -16,9 +16,6 @@ package configauth // import "go.opentelemetry.io/collector/config/configauth"
 
 import (
 	"context"
-	"net/http"
-
-	"google.golang.org/grpc"
 
 	"go.opentelemetry.io/collector/component"
 )
@@ -32,10 +29,6 @@ var (
 type MockServerAuthenticator struct {
 	// AuthenticateFunc to use during the authentication phase of this mock. Optional.
 	AuthenticateFunc AuthenticateFunc
-
-	// HTTPInterceptor to use in the test
-	HTTPInterceptorFunc HTTPInterceptorFunc
-	// TODO: implement the other funcs
 }
 
 // Authenticate executes the mock's AuthenticateFunc, if provided, or just returns the given context unchanged.
@@ -46,24 +39,6 @@ func (m *MockServerAuthenticator) Authenticate(ctx context.Context, headers map[
 	return m.AuthenticateFunc(ctx, headers)
 }
 
-// GRPCUnaryServerInterceptor isn't currently implemented and always returns nil.
-func (m *MockServerAuthenticator) GRPCUnaryServerInterceptor(context.Context, interface{}, *grpc.UnaryServerInfo, grpc.UnaryHandler) (interface{}, error) {
-	return nil, nil
-}
-
-// GRPCStreamServerInterceptor isn't currently implemented and always returns nil.
-func (m *MockServerAuthenticator) GRPCStreamServerInterceptor(interface{}, grpc.ServerStream, *grpc.StreamServerInfo, grpc.StreamHandler) error {
-	return nil
-}
-
-// HTTPInterceptor isn't currently implemented and always returns nil.
-func (m *MockServerAuthenticator) HTTPInterceptor(next http.Handler) http.Handler {
-	if m.HTTPInterceptorFunc == nil {
-		return next
-	}
-	return m.HTTPInterceptorFunc(next, m.AuthenticateFunc)
-}
-
 // Start isn't currently implemented and always returns nil.
 func (m *MockServerAuthenticator) Start(context.Context, component.Host) error {
 	return nil

diff --git a/config/configauth/mock_serverauth_test.go b/config/configauth/mock_serverauth_test.go
@@ -52,17 +52,6 @@ func TestNilOperations(t *testing.T) {
 		assert.NotNil(t, ctx)
 	}
 
-	{
-		ret, err := m.GRPCUnaryServerInterceptor(origCtx, nil, nil, nil)
-		assert.Nil(t, ret)
-		assert.NoError(t, err)
-	}
-
-	{
-		err := m.GRPCStreamServerInterceptor(nil, nil, nil, nil)
-		assert.NoError(t, err)
-	}
-
 	{
 		err := m.Start(origCtx, nil)
 		assert.NoError(t, err)

diff --git a/config/configauth/serverauth.go b/config/configauth/serverauth.go
@@ -16,18 +16,8 @@ package configauth // import "go.opentelemetry.io/collector/config/configauth"
 
 import (
 	"context"
-	"errors"
-	"net/http"
-
-	"google.golang.org/grpc"
-	"google.golang.org/grpc/metadata"
 
 	"go.opentelemetry.io/collector/component"
-	"go.opentelemetry.io/collector/internal/middleware"
-)
-
-var (
-	errMetadataNotFound = errors.New("no request metadata found")
 )
 
 // ServerAuthenticator is an Extension that can be used as an authenticator for the configauth.Authentication option.
@@ -47,90 +37,8 @@ type ServerAuthenticator interface {
 	// on tenancy as determined by the group membership, or passing through the authentication data to the next collector/backend.
 	// The context keys to be used are not defined yet.
 	Authenticate(ctx context.Context, headers map[string][]string) (context.Context, error)
-
-	// GRPCUnaryServerInterceptor is a helper method to provide a gRPC-compatible UnaryServerInterceptor, typically calling the authenticator's Authenticate method.
-	// While the context is the typical source of authentication data, the interceptor is free to determine where the auth data should come from. For instance, some
-	// receivers might implement an interceptor that looks into the payload instead.
-	// Once the authentication succeeds, the interceptor is expected to call the handler.
-	// See https://pkg.go.dev/google.golang.org/grpc#UnaryServerInterceptor.
-	GRPCUnaryServerInterceptor(ctx context.Context, req interface{}, srvInfo *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error)
-
-	// GRPCStreamServerInterceptor is a helper method to provide a gRPC-compatible StreamServerInterceptor, typically calling the authenticator's Authenticate method.
-	// While the context is the typical source of authentication data, the interceptor is free to determine where the auth data should come from. For instance, some
-	// receivers might implement an interceptor that looks into the payload instead.
-	// Once the authentication succeeds, the interceptor is expected to call the handler.
-	// See https://pkg.go.dev/google.golang.org/grpc#StreamServerInterceptor.
-	GRPCStreamServerInterceptor(srv interface{}, stream grpc.ServerStream, srvInfo *grpc.StreamServerInfo, handler grpc.StreamHandler) error
-
-	// HTTPInterceptor is a helper method to provide an HTTP handler responsible for intercepting the incoming HTTP requests, using the
-	// request's meta data as source of data for the authentication. Once the authentication succeeds, the interceptor is expected to call
-	// the next handler.
-	HTTPInterceptor(next http.Handler) http.Handler
 }
 
 // AuthenticateFunc defines the signature for the function responsible for performing the authentication based on the given headers map.
 // See ServerAuthenticator.Authenticate.
 type AuthenticateFunc func(ctx context.Context, headers map[string][]string) (context.Context, error)
-
-// GRPCUnaryInterceptorFunc defines the signature for the function intercepting unary gRPC calls, useful for authenticators to use as
-// types for internal structs, making it easier to mock them in tests.
-// See ServerAuthenticator.GRPCUnaryServerInterceptor.
-type GRPCUnaryInterceptorFunc func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler, authenticate AuthenticateFunc) (interface{}, error)
-
-// GRPCStreamInterceptorFunc defines the signature for the function intercepting streaming gRPC calls, useful for authenticators to use as
-// types for internal structs, making it easier to mock them in tests.
-// See ServerAuthenticator.GRPCStreamServerInterceptor.
-type GRPCStreamInterceptorFunc func(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler, authenticate AuthenticateFunc) error
-
-// HTTPInterceptorFunc defines the signature for the function intercepting HTTP calls, useful for authenticators to use as
-// types for internal structs, making it easier to mock them in tests.
-type HTTPInterceptorFunc func(handler http.Handler, authenticate AuthenticateFunc) http.Handler
-
-// DefaultGRPCUnaryServerInterceptor provides a default implementation of GRPCUnaryInterceptorFunc, useful for most authenticators.
-// It extracts the headers from the incoming request, under the assumption that the credentials will be part of the resulting map.
-func DefaultGRPCUnaryServerInterceptor(ctx context.Context, req interface{}, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler, authenticate AuthenticateFunc) (interface{}, error) {
-	headers, ok := metadata.FromIncomingContext(ctx)
-	if !ok {
-		return nil, errMetadataNotFound
-	}
-
-	ctx, err := authenticate(ctx, headers)
-	if err != nil {
-		return nil, err
-	}
-
-	return handler(ctx, req)
-}
-
-// DefaultGRPCStreamServerInterceptor provides a default implementation of GRPCStreamInterceptorFunc, useful for most authenticators.
-// It extracts the headers from the incoming request, under the assumption that the credentials will be part of the resulting map.
-func DefaultGRPCStreamServerInterceptor(srv interface{}, stream grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler, authenticate AuthenticateFunc) error {
-	ctx := stream.Context()
-	headers, ok := metadata.FromIncomingContext(ctx)
-	if !ok {
-		return errMetadataNotFound
-	}
-
-	ctx, err := authenticate(ctx, headers)
-	if err != nil {
-		return err
-	}
-
-	wrapped := middleware.WrapServerStream(stream)
-	wrapped.WrappedContext = ctx
-	return handler(srv, wrapped)
-}
-
-// DefaultHTTPInterceptor provides a default implementation of HTTPInterceptorFunc, useful for most authenticators.
-// It passes the headers from the incoming request as it is, under the assumption that the credentials are part of it.
-func DefaultHTTPInterceptor(next http.Handler, authenticate AuthenticateFunc) http.Handler {
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		ctx, err := authenticate(r.Context(), r.Header)
-		if err != nil {
-			http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
-			return
-		}
-
-		next.ServeHTTP(w, r.WithContext(ctx))
-	})
-}
diff --git a/config/configauth/serverauth_test.go b/config/configauth/serverauth_test.go