fix: middleware plugins memory leak

Co-authored-by: Julien Salleyron <julien.salleyron@gmail.com>

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
+	middlewareBuilders map[string]*middlewareBuilder
-	providerDescriptors   map[string]pluginContext
+	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{},
+		middlewareBuilders: map[string]*middlewareBuilder{},
-		providerDescriptors:   map[string]pluginContext{},
+		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{
+			middleware, err := newMiddlewareBuilder(i, manifest.BasePkg, manifest.Import)
-				interpreter: i,
+			if err != nil {
-				GoPath:      client.GoPath(),
+				return nil, err
-				Import:      manifest.Import,
-				BasePkg:     manifest.BasePkg,
 			}
+
+			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{
+			middleware, err := newMiddlewareBuilder(i, manifest.BasePkg, manifest.Import)
-				interpreter: i,
+			if err != nil {
-				GoPath:      localGoPath,
+				return nil, err
-				Import:      manifest.Import,
-				BasePkg:     manifest.BasePkg,
 			}
+
+			pb.middlewareBuilders[pName] = middleware
 		case "provider":
-			pb.providerDescriptors[pName] = pluginContext{
+			pb.providerBuilders[pName] = providerBuilder{
 				interpreter: i,
-				GoPath:      localGoPath,
 				Import:      manifest.Import,
 				BasePkg:     manifest.BasePkg,
 			}

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 middlewareBuilder struct {
-type Middleware struct {
-	middlewareName string
 	fnNew          reflect.Value
-	config         reflect.Value
+	fnCreateConfig reflect.Value
 }
 
-func newMiddleware(descriptor pluginContext, config map[string]interface{}, middlewareName string) (*Middleware, error) {
+func newMiddlewareBuilder(i *interp.Interpreter, basePkg, imp string) (*middlewareBuilder, error) {
-	basePkg := descriptor.BasePkg
 	if basePkg == "" {
-		basePkg = strings.ReplaceAll(path.Base(descriptor.Import), "-", "_")
+		basePkg = strings.ReplaceAll(path.Base(imp), "-", "_")
 	}
 
-	vConfig, err := descriptor.interpreter.Eval(basePkg + `.CreateConfig()`)
+	fnNew, err := i.Eval(basePkg + `.New`)
-	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`)
 	if err != nil {
 		return nil, fmt.Errorf("failed to eval New: %w", err)
 	}
 
-	return &Middleware{
+	fnCreateConfig, err := i.Eval(basePkg + `.CreateConfig`)
-		middlewareName: middlewareName,
+	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 (p middlewareBuilder) newHandler(ctx context.Context, next http.Handler, cfg reflect.Value, middlewareName string) (http.Handler, error) {
-func (m *Middleware) NewHandler(ctx context.Context, next http.Handler) (http.Handler, error) {
+	args := []reflect.Value{reflect.ValueOf(ctx), reflect.ValueOf(next), cfg, reflect.ValueOf(middlewareName)}
-	args := []reflect.Value{reflect.ValueOf(ctx), reflect.ValueOf(next), m.config, reflect.ValueOf(m.middlewareName)}
+	results := p.fnNew.Call(args)
-	results := m.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)
+}

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) {
+func newProvider(builder providerBuilder, config map[string]interface{}, providerName string) (*Provider, error) {
-	basePkg := descriptor.BasePkg
+	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)
 	}