From cc80568d9e60d04b97318c9b36e909d6355dc11d Mon Sep 17 00:00:00 2001 From: Julien Salleyron Date: Tue, 19 Nov 2024 14:52:04 +0100 Subject: [PATCH] Fix internal handlers ServiceBuilder composition --- pkg/server/service/internalhandler.go | 23 +++++------- pkg/server/service/managerfactory.go | 7 ++-- pkg/server/service/service.go | 25 +++++++++++-- pkg/server/service/service_test.go | 52 ++++++++++++++++++++++++--- 4 files changed, 80 insertions(+), 27 deletions(-) diff --git a/pkg/server/service/internalhandler.go b/pkg/server/service/internalhandler.go index 4bd6fd258..4ec26c930 100644 --- a/pkg/server/service/internalhandler.go +++ b/pkg/server/service/internalhandler.go @@ -8,11 +8,6 @@ import ( "strings" ) -type serviceManager interface { - BuildHTTP(rootCtx context.Context, serviceName string) (http.Handler, error) - LaunchHealthCheck() -} - // InternalHandlers is the internal HTTP handlers builder. type InternalHandlers struct { api http.Handler @@ -21,26 +16,24 @@ type InternalHandlers struct { prometheus http.Handler ping http.Handler acmeHTTP http.Handler - serviceManager } // NewInternalHandlers creates a new InternalHandlers. -func NewInternalHandlers(next serviceManager, apiHandler, rest, metricsHandler, pingHandler, dashboard, acmeHTTP http.Handler) *InternalHandlers { +func NewInternalHandlers(apiHandler, rest, metricsHandler, pingHandler, dashboard, acmeHTTP http.Handler) *InternalHandlers { return &InternalHandlers{ - api: apiHandler, - dashboard: dashboard, - rest: rest, - prometheus: metricsHandler, - ping: pingHandler, - acmeHTTP: acmeHTTP, - serviceManager: next, + api: apiHandler, + dashboard: dashboard, + rest: rest, + prometheus: metricsHandler, + ping: pingHandler, + acmeHTTP: acmeHTTP, } } // BuildHTTP builds an HTTP handler. func (m *InternalHandlers) BuildHTTP(rootCtx context.Context, serviceName string) (http.Handler, error) { if !strings.HasSuffix(serviceName, "@internal") { - return m.serviceManager.BuildHTTP(rootCtx, serviceName) + return nil, nil } internalHandler, err := m.get(serviceName) diff --git a/pkg/server/service/managerfactory.go b/pkg/server/service/managerfactory.go index 5db06ef40..07ba4e25d 100644 --- a/pkg/server/service/managerfactory.go +++ b/pkg/server/service/managerfactory.go @@ -71,13 +71,12 @@ func NewManagerFactory(staticConfiguration static.Configuration, routinesPool *s } // Build creates a service manager. -func (f *ManagerFactory) Build(configuration *runtime.Configuration) *InternalHandlers { - svcManager := NewManager(configuration.Services, f.metricsRegistry, f.routinesPool, f.roundTripperManager) - +func (f *ManagerFactory) Build(configuration *runtime.Configuration) *Manager { var apiHandler http.Handler if f.api != nil { apiHandler = f.api(configuration) } - return NewInternalHandlers(svcManager, apiHandler, f.restHandler, f.metricsHandler, f.pingHandler, f.dashboardHandler, f.acmeHTTPHandler) + internalHandlers := NewInternalHandlers(apiHandler, f.restHandler, f.metricsHandler, f.pingHandler, f.dashboardHandler, f.acmeHTTPHandler) + return NewManager(configuration.Services, f.metricsRegistry, f.routinesPool, f.roundTripperManager, internalHandlers) } diff --git a/pkg/server/service/service.go b/pkg/server/service/service.go index cc43e76a2..f30770b62 100644 --- a/pkg/server/service/service.go +++ b/pkg/server/service/service.go @@ -34,22 +34,28 @@ import ( const ( defaultHealthCheckInterval = 30 * time.Second defaultHealthCheckTimeout = 5 * time.Second -) -const defaultMaxBodySize int64 = -1 + defaultMaxBodySize int64 = -1 +) // RoundTripperGetter is a roundtripper getter interface. type RoundTripperGetter interface { Get(name string) (http.RoundTripper, error) } +// ServiceBuilder is a Service builder. +type ServiceBuilder interface { + BuildHTTP(rootCtx context.Context, serviceName string) (http.Handler, error) +} + // NewManager creates a new Manager. -func NewManager(configs map[string]*runtime.ServiceInfo, metricsRegistry metrics.Registry, routinePool *safe.Pool, roundTripperManager RoundTripperGetter) *Manager { +func NewManager(configs map[string]*runtime.ServiceInfo, metricsRegistry metrics.Registry, routinePool *safe.Pool, roundTripperManager RoundTripperGetter, serviceBuilders ...ServiceBuilder) *Manager { return &Manager{ routinePool: routinePool, metricsRegistry: metricsRegistry, bufferPool: newBufferPool(), roundTripperManager: roundTripperManager, + serviceBuilders: serviceBuilders, balancers: make(map[string]healthcheck.Balancers), configs: configs, rand: rand.New(rand.NewSource(time.Now().UnixNano())), @@ -62,6 +68,8 @@ type Manager struct { metricsRegistry metrics.Registry bufferPool httputil.BufferPool roundTripperManager RoundTripperGetter + serviceBuilders []ServiceBuilder + // balancers is the map of all Balancers, keyed by service name. // There is one Balancer per service handler, and there is one service handler per reference to a service // (e.g. if 2 routers refer to the same service name, 2 service handlers are created), @@ -78,6 +86,17 @@ func (m *Manager) BuildHTTP(rootCtx context.Context, serviceName string) (http.H serviceName = provider.GetQualifiedName(ctx, serviceName) ctx = provider.AddInContext(ctx, serviceName) + // Must be before we get configs to handle services without config. + for _, builder := range m.serviceBuilders { + handler, err := builder.BuildHTTP(rootCtx, serviceName) + if err != nil { + return nil, err + } + if handler != nil { + return handler, nil + } + } + conf, ok := m.configs[serviceName] if !ok { return nil, fmt.Errorf("the service %q does not exist", serviceName) diff --git a/pkg/server/service/service_test.go b/pkg/server/service/service_test.go index 8b6b305a9..7e73de2d2 100644 --- a/pkg/server/service/service_test.go +++ b/pkg/server/service/service_test.go @@ -18,9 +18,9 @@ import ( "github.com/traefik/traefik/v2/pkg/testhelpers" ) -type MockForwarder struct{} +type mockForwarder struct{} -func (MockForwarder) ServeHTTP(http.ResponseWriter, *http.Request) { +func (mockForwarder) ServeHTTP(http.ResponseWriter, *http.Request) { panic("implement me") } @@ -44,14 +44,14 @@ func TestGetLoadBalancer(t *testing.T) { }, }, }, - fwd: &MockForwarder{}, + fwd: &mockForwarder{}, expectError: true, }, { desc: "Succeeds when there are no servers", serviceName: "test", service: &dynamic.ServersLoadBalancer{}, - fwd: &MockForwarder{}, + fwd: &mockForwarder{}, expectError: false, }, { @@ -60,7 +60,7 @@ func TestGetLoadBalancer(t *testing.T) { service: &dynamic.ServersLoadBalancer{ Sticky: &dynamic.Sticky{Cookie: &dynamic.Cookie{}}, }, - fwd: &MockForwarder{}, + fwd: &mockForwarder{}, expectError: false, }, } @@ -476,6 +476,48 @@ func Test1xxResponses(t *testing.T) { } } +type serviceBuilderFunc func(ctx context.Context, serviceName string) (http.Handler, error) + +func (s serviceBuilderFunc) BuildHTTP(ctx context.Context, serviceName string) (http.Handler, error) { + return s(ctx, serviceName) +} + +type internalHandler struct{} + +func (internalHandler) ServeHTTP(_ http.ResponseWriter, _ *http.Request) {} + +func TestManager_ServiceBuilders(t *testing.T) { + var internalHandler internalHandler + + manager := NewManager(map[string]*runtime.ServiceInfo{ + "test@test": { + Service: &dynamic.Service{ + LoadBalancer: &dynamic.ServersLoadBalancer{}, + }, + }, + }, nil, nil, &RoundTripperManager{ + roundTrippers: map[string]http.RoundTripper{ + "default@internal": http.DefaultTransport, + }, + }, serviceBuilderFunc(func(rootCtx context.Context, serviceName string) (http.Handler, error) { + if strings.HasSuffix(serviceName, "@internal") { + return internalHandler, nil + } + return nil, nil + })) + + h, err := manager.BuildHTTP(context.Background(), "test@internal") + require.NoError(t, err) + assert.Equal(t, internalHandler, h) + + h, err = manager.BuildHTTP(context.Background(), "test@test") + require.NoError(t, err) + assert.NotNil(t, h) + + _, err = manager.BuildHTTP(context.Background(), "wrong@test") + assert.Error(t, err) +} + func TestManager_Build(t *testing.T) { testCases := []struct { desc string