From 577709fff345e7860da215896e35ff9333449394 Mon Sep 17 00:00:00 2001 From: Ludovic Fernandez Date: Fri, 14 Jan 2022 12:22:06 +0100 Subject: [PATCH] fix: middleware plugins memory leak Co-authored-by: Julien Salleyron --- pkg/plugins/builder.go | 48 ++++++---------- pkg/plugins/middlewares.go | 114 ++++++++++++++++++++++++------------- pkg/plugins/providers.go | 31 ++++++---- 3 files changed, 112 insertions(+), 81 deletions(-) diff --git a/pkg/plugins/builder.go b/pkg/plugins/builder.go index f3609dd0e..cf99b0e6b 100644 --- a/pkg/plugins/builder.go +++ b/pkg/plugins/builder.go @@ -15,31 +15,17 @@ import ( // Constructor creates a plugin handler. type Constructor func(context.Context, http.Handler) (http.Handler, error) -// pluginContext The static part of a plugin configuration. -type pluginContext struct { - // GoPath plugin's GOPATH - GoPath string `json:"goPath,omitempty" toml:"goPath,omitempty" yaml:"goPath,omitempty"` - - // Import plugin's import/package - Import string `json:"import,omitempty" toml:"import,omitempty" yaml:"import,omitempty"` - - // BasePkg plugin's base package name (optional) - BasePkg string `json:"basePkg,omitempty" toml:"basePkg,omitempty" yaml:"basePkg,omitempty"` - - interpreter *interp.Interpreter -} - // Builder is a plugin builder. type Builder struct { - middlewareDescriptors map[string]pluginContext - providerDescriptors map[string]pluginContext + middlewareBuilders map[string]*middlewareBuilder + providerBuilders map[string]providerBuilder } // NewBuilder creates a new Builder. func NewBuilder(client *Client, plugins map[string]Descriptor, localPlugins map[string]LocalDescriptor) (*Builder, error) { pb := &Builder{ - middlewareDescriptors: map[string]pluginContext{}, - providerDescriptors: map[string]pluginContext{}, + middlewareBuilders: map[string]*middlewareBuilder{}, + providerBuilders: map[string]providerBuilder{}, } for pName, desc := range plugins { @@ -74,16 +60,15 @@ func NewBuilder(client *Client, plugins map[string]Descriptor, localPlugins map[ switch manifest.Type { case "middleware": - pb.middlewareDescriptors[pName] = pluginContext{ - interpreter: i, - GoPath: client.GoPath(), - Import: manifest.Import, - BasePkg: manifest.BasePkg, + middleware, err := newMiddlewareBuilder(i, manifest.BasePkg, manifest.Import) + if err != nil { + return nil, err } + + pb.middlewareBuilders[pName] = middleware case "provider": - pb.providerDescriptors[pName] = pluginContext{ + pb.providerBuilders[pName] = providerBuilder{ interpreter: i, - GoPath: client.GoPath(), Import: manifest.Import, BasePkg: manifest.BasePkg, } @@ -123,16 +108,15 @@ func NewBuilder(client *Client, plugins map[string]Descriptor, localPlugins map[ switch manifest.Type { case "middleware": - pb.middlewareDescriptors[pName] = pluginContext{ - interpreter: i, - GoPath: localGoPath, - Import: manifest.Import, - BasePkg: manifest.BasePkg, + middleware, err := newMiddlewareBuilder(i, manifest.BasePkg, manifest.Import) + if err != nil { + return nil, err } + + pb.middlewareBuilders[pName] = middleware case "provider": - pb.providerDescriptors[pName] = pluginContext{ + pb.providerBuilders[pName] = providerBuilder{ interpreter: i, - GoPath: localGoPath, Import: manifest.Import, BasePkg: manifest.BasePkg, } diff --git a/pkg/plugins/middlewares.go b/pkg/plugins/middlewares.go index e9011de73..09edbc954 100644 --- a/pkg/plugins/middlewares.go +++ b/pkg/plugins/middlewares.go @@ -9,15 +9,16 @@ import ( "strings" "github.com/mitchellh/mapstructure" + "github.com/traefik/yaegi/interp" ) // Build builds a middleware plugin. func (b Builder) Build(pName string, config map[string]interface{}, middlewareName string) (Constructor, error) { - if b.middlewareDescriptors == nil { + if b.middlewareBuilders == nil { return nil, fmt.Errorf("no plugin definition in the static configuration: %s", pName) } - descriptor, ok := b.middlewareDescriptors[pName] + descriptor, ok := b.middlewareBuilders[pName] if !ok { return nil, fmt.Errorf("unknown plugin type: %s", pName) } @@ -30,59 +31,42 @@ func (b Builder) Build(pName string, config map[string]interface{}, middlewareNa return m.NewHandler, err } -// Middleware is a HTTP handler plugin wrapper. -type Middleware struct { - middlewareName string +type middlewareBuilder struct { fnNew reflect.Value - config reflect.Value + fnCreateConfig reflect.Value } -func newMiddleware(descriptor pluginContext, config map[string]interface{}, middlewareName string) (*Middleware, error) { - basePkg := descriptor.BasePkg +func newMiddlewareBuilder(i *interp.Interpreter, basePkg, imp string) (*middlewareBuilder, error) { if basePkg == "" { - basePkg = strings.ReplaceAll(path.Base(descriptor.Import), "-", "_") + basePkg = strings.ReplaceAll(path.Base(imp), "-", "_") } - vConfig, err := descriptor.interpreter.Eval(basePkg + `.CreateConfig()`) - if err != nil { - return nil, fmt.Errorf("failed to eval CreateConfig: %w", err) - } - - cfg := &mapstructure.DecoderConfig{ - DecodeHook: mapstructure.StringToSliceHookFunc(","), - WeaklyTypedInput: true, - Result: vConfig.Interface(), - } - - decoder, err := mapstructure.NewDecoder(cfg) - if err != nil { - return nil, fmt.Errorf("failed to create configuration decoder: %w", err) - } - - err = decoder.Decode(config) - if err != nil { - return nil, fmt.Errorf("failed to decode configuration: %w", err) - } - - fnNew, err := descriptor.interpreter.Eval(basePkg + `.New`) + fnNew, err := i.Eval(basePkg + `.New`) if err != nil { return nil, fmt.Errorf("failed to eval New: %w", err) } - return &Middleware{ - middlewareName: middlewareName, + fnCreateConfig, err := i.Eval(basePkg + `.CreateConfig`) + if err != nil { + return nil, fmt.Errorf("failed to eval CreateConfig: %w", err) + } + + return &middlewareBuilder{ fnNew: fnNew, - config: vConfig, + fnCreateConfig: fnCreateConfig, }, nil } -// NewHandler creates a new HTTP handler. -func (m *Middleware) NewHandler(ctx context.Context, next http.Handler) (http.Handler, error) { - args := []reflect.Value{reflect.ValueOf(ctx), reflect.ValueOf(next), m.config, reflect.ValueOf(m.middlewareName)} - results := m.fnNew.Call(args) +func (p middlewareBuilder) newHandler(ctx context.Context, next http.Handler, cfg reflect.Value, middlewareName string) (http.Handler, error) { + args := []reflect.Value{reflect.ValueOf(ctx), reflect.ValueOf(next), cfg, reflect.ValueOf(middlewareName)} + results := p.fnNew.Call(args) if len(results) > 1 && results[1].Interface() != nil { - return nil, results[1].Interface().(error) + err, ok := results[1].Interface().(error) + if !ok { + return nil, fmt.Errorf("invalid error type: %T", results[0].Interface()) + } + return nil, err } handler, ok := results[0].Interface().(http.Handler) @@ -92,3 +76,55 @@ func (m *Middleware) NewHandler(ctx context.Context, next http.Handler) (http.Ha return handler, nil } + +func (p middlewareBuilder) createConfig(config map[string]interface{}) (reflect.Value, error) { + results := p.fnCreateConfig.Call(nil) + if len(results) != 1 { + return reflect.Value{}, fmt.Errorf("invalid number of return for the CreateConfig function: %d", len(results)) + } + + vConfig := results[0] + + cfg := &mapstructure.DecoderConfig{ + DecodeHook: mapstructure.StringToSliceHookFunc(","), + WeaklyTypedInput: true, + Result: vConfig.Interface(), + } + + decoder, err := mapstructure.NewDecoder(cfg) + if err != nil { + return reflect.Value{}, fmt.Errorf("failed to create configuration decoder: %w", err) + } + + err = decoder.Decode(config) + if err != nil { + return reflect.Value{}, fmt.Errorf("failed to decode configuration: %w", err) + } + + return vConfig, nil +} + +// Middleware is an HTTP handler plugin wrapper. +type Middleware struct { + middlewareName string + config reflect.Value + builder *middlewareBuilder +} + +func newMiddleware(builder *middlewareBuilder, config map[string]interface{}, middlewareName string) (*Middleware, error) { + vConfig, err := builder.createConfig(config) + if err != nil { + return nil, err + } + + return &Middleware{ + middlewareName: middlewareName, + config: vConfig, + builder: builder, + }, nil +} + +// NewHandler creates a new HTTP handler. +func (m *Middleware) NewHandler(ctx context.Context, next http.Handler) (http.Handler, error) { + return m.builder.newHandler(ctx, next, m.config, m.middlewareName) +} diff --git a/pkg/plugins/providers.go b/pkg/plugins/providers.go index bccc998cf..abec8d1b8 100644 --- a/pkg/plugins/providers.go +++ b/pkg/plugins/providers.go @@ -13,6 +13,7 @@ import ( "github.com/traefik/traefik/v2/pkg/log" "github.com/traefik/traefik/v2/pkg/provider" "github.com/traefik/traefik/v2/pkg/safe" + "github.com/traefik/yaegi/interp" ) // PP the interface of a plugin's provider. @@ -52,16 +53,26 @@ func ppSymbols() map[string]map[string]reflect.Value { // BuildProvider builds a plugin's provider. func (b Builder) BuildProvider(pName string, config map[string]interface{}) (provider.Provider, error) { - if b.providerDescriptors == nil { + if b.providerBuilders == nil { return nil, fmt.Errorf("no plugin definition in the static configuration: %s", pName) } - descriptor, ok := b.providerDescriptors[pName] + builder, ok := b.providerBuilders[pName] if !ok { return nil, fmt.Errorf("unknown plugin type: %s", pName) } - return newProvider(descriptor, config, "plugin-"+pName) + return newProvider(builder, config, "plugin-"+pName) +} + +type providerBuilder struct { + // Import plugin's import/package + Import string `json:"import,omitempty" toml:"import,omitempty" yaml:"import,omitempty"` + + // BasePkg plugin's base package name (optional) + BasePkg string `json:"basePkg,omitempty" toml:"basePkg,omitempty" yaml:"basePkg,omitempty"` + + interpreter *interp.Interpreter } // Provider is a plugin's provider wrapper. @@ -70,13 +81,13 @@ type Provider struct { pp PP } -func newProvider(descriptor pluginContext, config map[string]interface{}, providerName string) (*Provider, error) { - basePkg := descriptor.BasePkg +func newProvider(builder providerBuilder, config map[string]interface{}, providerName string) (*Provider, error) { + basePkg := builder.BasePkg if basePkg == "" { - basePkg = strings.ReplaceAll(path.Base(descriptor.Import), "-", "_") + basePkg = strings.ReplaceAll(path.Base(builder.Import), "-", "_") } - vConfig, err := descriptor.interpreter.Eval(basePkg + `.CreateConfig()`) + vConfig, err := builder.interpreter.Eval(basePkg + `.CreateConfig()`) if err != nil { return nil, fmt.Errorf("failed to eval CreateConfig: %w", err) } @@ -97,12 +108,12 @@ func newProvider(descriptor pluginContext, config map[string]interface{}, provid return nil, fmt.Errorf("failed to decode configuration: %w", err) } - _, err = descriptor.interpreter.Eval(`package wrapper + _, err = builder.interpreter.Eval(`package wrapper import ( "context" - ` + basePkg + ` "` + descriptor.Import + `" + ` + basePkg + ` "` + builder.Import + `" "github.com/traefik/traefik/v2/pkg/plugins" ) @@ -116,7 +127,7 @@ func NewWrapper(ctx context.Context, config *` + basePkg + `.Config, name string return nil, fmt.Errorf("failed to eval wrapper: %w", err) } - fnNew, err := descriptor.interpreter.Eval("wrapper.NewWrapper") + fnNew, err := builder.interpreter.Eval("wrapper.NewWrapper") if err != nil { return nil, fmt.Errorf("failed to eval New: %w", err) }