From 8735263930d6086918ca720f768fbe13e7be4c5a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9rald=20Cro=C3=ABs?= Date: Tue, 15 Jan 2019 05:28:04 -0800 Subject: [PATCH] Enables the use of elements declared in other providers --- integration/fixtures/multiprovider.toml | 24 ++ integration/simple_test.go | 53 ++++ integration/tracing_test.go | 6 +- middlewares/chain/chain.go | 8 +- server/aggregator.go | 29 ++ server/aggregator_test.go | 103 +++++++ server/internal/provider.go | 45 +++ server/middleware/middlewares.go | 59 +++- server/middleware/middlewares_test.go | 261 +++++++++++++++++- server/router/route_appender_aggregator.go | 61 ++-- .../router/route_appender_aggregator_test.go | 6 +- server/router/router.go | 10 +- server/router/router_test.go | 96 +++++++ server/server_configuration.go | 23 +- server/service/service.go | 7 +- server/service/service_test.go | 56 ++++ 16 files changed, 753 insertions(+), 94 deletions(-) create mode 100644 integration/fixtures/multiprovider.toml create mode 100644 server/aggregator.go create mode 100644 server/aggregator_test.go create mode 100644 server/internal/provider.go diff --git a/integration/fixtures/multiprovider.toml b/integration/fixtures/multiprovider.toml new file mode 100644 index 000000000..1262935a5 --- /dev/null +++ b/integration/fixtures/multiprovider.toml @@ -0,0 +1,24 @@ +[log] +logLevel = "DEBUG" + +[api] + +[entryPoints] + [entryPoints.http] + address = ":8000" + +[Providers] + [Providers.Rest] + + [Providers.File] + +[Services] + [Services.service] + [Services.service.LoadBalancer] + + [[Services.service.LoadBalancer.Servers]] + URL = "{{.Server}}" + Weight = 1 +[Middlewares] + [Middlewares.customheader.Headers.CustomRequestHeaders] + X-Custom="CustomValue" diff --git a/integration/simple_test.go b/integration/simple_test.go index abdd5284e..f1e4e541f 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -1,6 +1,8 @@ package integration import ( + "bytes" + "encoding/json" "fmt" "net/http" "net/http/httptest" @@ -9,6 +11,7 @@ import ( "syscall" "time" + "github.com/containous/traefik/config" "github.com/containous/traefik/integration/try" "github.com/go-check/check" checker "github.com/vdemeester/shakers" @@ -434,3 +437,53 @@ func (s *SimpleSuite) TestKeepTrailingSlash(c *check.C) { http.DefaultClient.CheckRedirect = oldCheckRedirect } + +func (s *SimpleSuite) TestMultiprovider(c *check.C) { + s.createComposeProject(c, "base") + s.composeProject.Start(c) + + server := "http://" + s.composeProject.Container(c, "whoami1").NetworkSettings.IPAddress + + file := s.adaptFile(c, "fixtures/multiprovider.toml", struct { + Server string + }{Server: server}) + defer os.Remove(file) + + cmd, output := s.traefikCmd(withConfigFile(file)) + defer output(c) + + err := cmd.Start() + c.Assert(err, checker.IsNil) + defer cmd.Process.Kill() + + err = try.GetRequest("http://127.0.0.1:8080/api/providers/file/services", 1000*time.Millisecond, try.BodyContains("service")) + c.Assert(err, checker.IsNil) + + config := config.Configuration{ + Routers: map[string]*config.Router{ + "router1": { + EntryPoints: []string{"http"}, + Middlewares: []string{"file.customheader"}, + Service: "file.service", + Rule: "PathPrefix:/", + }, + }, + } + + json, err := json.Marshal(config) + c.Assert(err, checker.IsNil) + + request, err := http.NewRequest(http.MethodPut, "http://127.0.0.1:8080/api/providers/rest", bytes.NewReader(json)) + c.Assert(err, checker.IsNil) + + response, err := http.DefaultClient.Do(request) + c.Assert(err, checker.IsNil) + c.Assert(response.StatusCode, checker.Equals, http.StatusOK) + + err = try.GetRequest("http://127.0.0.1:8080/api/providers/rest/routers", 1000*time.Millisecond, try.BodyContains("PathPrefix:/")) + c.Assert(err, checker.IsNil) + + err = try.GetRequest("http://127.0.0.1:8000/", 1*time.Second, try.StatusCodeIs(http.StatusOK), try.BodyContains("CustomValue")) + c.Assert(err, checker.IsNil) + +} diff --git a/integration/tracing_test.go b/integration/tracing_test.go index 22c37bb62..0add8cef5 100644 --- a/integration/tracing_test.go +++ b/integration/tracing_test.go @@ -85,7 +85,7 @@ func (s *TracingSuite) TestZipkinRateLimit(c *check.C) { err = try.GetRequest("http://127.0.0.1:8000/ratelimit", 500*time.Millisecond, try.StatusCodeIs(http.StatusTooManyRequests)) c.Assert(err, checker.IsNil) - err = try.GetRequest("http://"+s.ZipkinIP+":9411/api/v2/spans?serviceName=tracing", 20*time.Second, try.BodyContains("forward service1/router1", "ratelimit")) + err = try.GetRequest("http://"+s.ZipkinIP+":9411/api/v2/spans?serviceName=tracing", 20*time.Second, try.BodyContains("forward service1/file.router1", "file.ratelimit")) c.Assert(err, checker.IsNil) } @@ -109,7 +109,7 @@ func (s *TracingSuite) TestZipkinRetry(c *check.C) { err = try.GetRequest("http://127.0.0.1:8000/retry", 500*time.Millisecond, try.StatusCodeIs(http.StatusBadGateway)) c.Assert(err, checker.IsNil) - err = try.GetRequest("http://"+s.ZipkinIP+":9411/api/v2/spans?serviceName=tracing", 20*time.Second, try.BodyContains("forward service2/router2", "retry")) + err = try.GetRequest("http://"+s.ZipkinIP+":9411/api/v2/spans?serviceName=tracing", 20*time.Second, try.BodyContains("forward service2/file.router2", "file.retry")) c.Assert(err, checker.IsNil) } @@ -132,6 +132,6 @@ func (s *TracingSuite) TestZipkinAuth(c *check.C) { err = try.GetRequest("http://127.0.0.1:8000/auth", 500*time.Millisecond, try.StatusCodeIs(http.StatusUnauthorized)) c.Assert(err, checker.IsNil) - err = try.GetRequest("http://"+s.ZipkinIP+":9411/api/v2/spans?serviceName=tracing", 20*time.Second, try.BodyContains("entrypoint http", "basic-auth")) + err = try.GetRequest("http://"+s.ZipkinIP+":9411/api/v2/spans?serviceName=tracing", 20*time.Second, try.BodyContains("entrypoint http", "file.basic-auth")) c.Assert(err, checker.IsNil) } diff --git a/middlewares/chain/chain.go b/middlewares/chain/chain.go index 3ba4208a0..ceb62e789 100644 --- a/middlewares/chain/chain.go +++ b/middlewares/chain/chain.go @@ -14,17 +14,13 @@ const ( ) type chainBuilder interface { - BuildChain(ctx context.Context, middlewares []string) (*alice.Chain, error) + BuildChain(ctx context.Context, middlewares []string) *alice.Chain } // New creates a chain middleware func New(ctx context.Context, next http.Handler, config config.Chain, builder chainBuilder, name string) (http.Handler, error) { middlewares.GetLogger(ctx, name, typeName).Debug("Creating middleware") - middlewareChain, err := builder.BuildChain(ctx, config.Middlewares) - if err != nil { - return nil, err - } - + middlewareChain := builder.BuildChain(ctx, config.Middlewares) return middlewareChain.Then(next) } diff --git a/server/aggregator.go b/server/aggregator.go new file mode 100644 index 000000000..0c39b6a34 --- /dev/null +++ b/server/aggregator.go @@ -0,0 +1,29 @@ +package server + +import ( + "github.com/containous/traefik/config" + "github.com/containous/traefik/server/internal" +) + +func mergeConfiguration(configurations config.Configurations) config.Configuration { + conf := config.Configuration{ + Routers: make(map[string]*config.Router), + Middlewares: make(map[string]*config.Middleware), + Services: make(map[string]*config.Service), + } + + for provider, configuration := range configurations { + for routerName, router := range configuration.Routers { + conf.Routers[internal.MakeQualifiedName(provider, routerName)] = router + } + for middlewareName, middleware := range configuration.Middlewares { + conf.Middlewares[internal.MakeQualifiedName(provider, middlewareName)] = middleware + } + for serviceName, service := range configuration.Services { + conf.Services[internal.MakeQualifiedName(provider, serviceName)] = service + } + conf.TLS = append(conf.TLS, configuration.TLS...) + } + + return conf +} diff --git a/server/aggregator_test.go b/server/aggregator_test.go new file mode 100644 index 000000000..420992349 --- /dev/null +++ b/server/aggregator_test.go @@ -0,0 +1,103 @@ +package server + +import ( + "testing" + + "github.com/containous/traefik/config" + "github.com/stretchr/testify/assert" +) + +func TestAggregator(t *testing.T) { + testCases := []struct { + desc string + given config.Configurations + expected config.Configuration + }{ + { + desc: "Nil returns an empty configuration", + given: nil, + expected: config.Configuration{ + Routers: make(map[string]*config.Router), + Middlewares: make(map[string]*config.Middleware), + Services: make(map[string]*config.Service), + }, + }, + { + desc: "Returns fully qualified elements from a mono-provider configuration map", + given: config.Configurations{ + "provider-1": &config.Configuration{ + Routers: map[string]*config.Router{ + "router-1": {}, + }, + Middlewares: map[string]*config.Middleware{ + "middleware-1": {}, + }, + Services: map[string]*config.Service{ + "service-1": {}, + }, + }, + }, + expected: config.Configuration{ + Routers: map[string]*config.Router{ + "provider-1.router-1": {}, + }, + Middlewares: map[string]*config.Middleware{ + "provider-1.middleware-1": {}, + }, + Services: map[string]*config.Service{ + "provider-1.service-1": {}, + }, + }, + }, + { + desc: "Returns fully qualified elements from a multi-provider configuration map", + given: config.Configurations{ + "provider-1": &config.Configuration{ + Routers: map[string]*config.Router{ + "router-1": {}, + }, + Middlewares: map[string]*config.Middleware{ + "middleware-1": {}, + }, + Services: map[string]*config.Service{ + "service-1": {}, + }, + }, + "provider-2": &config.Configuration{ + Routers: map[string]*config.Router{ + "router-1": {}, + }, + Middlewares: map[string]*config.Middleware{ + "middleware-1": {}, + }, + Services: map[string]*config.Service{ + "service-1": {}, + }, + }, + }, + expected: config.Configuration{ + Routers: map[string]*config.Router{ + "provider-1.router-1": {}, + "provider-2.router-1": {}, + }, + Middlewares: map[string]*config.Middleware{ + "provider-1.middleware-1": {}, + "provider-2.middleware-1": {}, + }, + Services: map[string]*config.Service{ + "provider-1.service-1": {}, + "provider-2.service-1": {}, + }, + }, + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + actual := mergeConfiguration(test.given) + assert.Equal(t, test.expected, actual) + }) + } +} diff --git a/server/internal/provider.go b/server/internal/provider.go new file mode 100644 index 000000000..0704068d5 --- /dev/null +++ b/server/internal/provider.go @@ -0,0 +1,45 @@ +package internal + +import ( + "context" + "strings" + + "github.com/containous/traefik/log" +) + +type contextKey int + +const ( + providerKey contextKey = iota +) + +// AddProviderInContext Adds the provider name in the context +func AddProviderInContext(ctx context.Context, elementName string) context.Context { + parts := strings.Split(elementName, ".") + if len(parts) == 1 { + log.FromContext(ctx).Debugf("Could not find a provider for %s.", elementName) + return ctx + } + + if name, ok := ctx.Value(providerKey).(string); ok && name == parts[0] { + return ctx + } + + return context.WithValue(ctx, providerKey, parts[0]) +} + +// GetQualifiedName Gets the fully qualified name. +func GetQualifiedName(ctx context.Context, elementName string) string { + parts := strings.Split(elementName, ".") + if len(parts) == 1 { + if providerName, ok := ctx.Value(providerKey).(string); ok { + return MakeQualifiedName(providerName, parts[0]) + } + } + return elementName +} + +// MakeQualifiedName Creates a qualified name for an element +func MakeQualifiedName(providerName string, elementName string) string { + return providerName + "." + elementName +} diff --git a/server/middleware/middlewares.go b/server/middleware/middlewares.go index b1b4d9f79..103b7676f 100644 --- a/server/middleware/middlewares.go +++ b/server/middleware/middlewares.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "strings" "github.com/containous/alice" "github.com/containous/traefik/config" @@ -26,9 +27,16 @@ import ( "github.com/containous/traefik/middlewares/stripprefix" "github.com/containous/traefik/middlewares/stripprefixregex" "github.com/containous/traefik/middlewares/tracing" + "github.com/containous/traefik/server/internal" "github.com/pkg/errors" ) +type middlewareStackType int + +const ( + middlewareStackKey middlewareStackType = iota +) + // Builder the middleware builder type Builder struct { configs map[string]*config.Middleware @@ -45,22 +53,40 @@ func NewBuilder(configs map[string]*config.Middleware, serviceBuilder serviceBui } // BuildChain creates a middleware chain -func (b *Builder) BuildChain(ctx context.Context, middlewares []string) (*alice.Chain, error) { +func (b *Builder) BuildChain(ctx context.Context, middlewares []string) *alice.Chain { chain := alice.New() for _, middlewareName := range middlewares { - if _, ok := b.configs[middlewareName]; !ok { - return nil, fmt.Errorf("middleware %q does not exist", middlewareName) - } + middlewareName := internal.GetQualifiedName(ctx, middlewareName) + constructorContext := internal.AddProviderInContext(ctx, middlewareName) + chain = chain.Append(func(next http.Handler) (http.Handler, error) { + var err error + if constructorContext, err = checkRecursivity(constructorContext, middlewareName); err != nil { + return nil, err + } - constructor, err := b.buildConstructor(ctx, middlewareName, *b.configs[middlewareName]) - if err != nil { - return nil, err - } - if constructor != nil { - chain = chain.Append(constructor) - } + if _, ok := b.configs[middlewareName]; !ok { + return nil, fmt.Errorf("middleware %q does not exist", middlewareName) + } + + constructor, err := b.buildConstructor(constructorContext, middlewareName, *b.configs[middlewareName]) + if err != nil { + return nil, fmt.Errorf("error while instanciation of %s: %v", middlewareName, err) + } + return constructor(next) + }) } - return &chain, nil + return &chain +} + +func checkRecursivity(ctx context.Context, middlewareName string) (context.Context, error) { + currentStack, ok := ctx.Value(middlewareStackKey).([]string) + if !ok { + currentStack = []string{} + } + if inSlice(middlewareName, currentStack) { + return ctx, fmt.Errorf("could not instantiate middleware %s: recursion detected in %s", middlewareName, strings.Join(append(currentStack, middlewareName), "->")) + } + return context.WithValue(ctx, middlewareStackKey, append(currentStack, middlewareName)), nil } func (b *Builder) buildConstructor(ctx context.Context, middlewareName string, config config.Middleware) (alice.Constructor, error) { @@ -290,3 +316,12 @@ func (b *Builder) buildConstructor(ctx context.Context, middlewareName string, c return tracing.Wrap(ctx, middleware), nil } + +func inSlice(element string, stack []string) bool { + for _, value := range stack { + if value == element { + return true + } + } + return false +} diff --git a/server/middleware/middlewares_test.go b/server/middleware/middlewares_test.go index 76a66d6c5..d3147f2a8 100644 --- a/server/middleware/middlewares_test.go +++ b/server/middleware/middlewares_test.go @@ -3,9 +3,13 @@ package middleware import ( "context" "net/http" + "net/http/httptest" "testing" "github.com/containous/traefik/config" + "github.com/containous/traefik/server/internal" + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -67,10 +71,8 @@ func TestMiddlewaresRegistry_BuildChainNilConfig(t *testing.T) { } middlewaresBuilder := NewBuilder(testConfig, nil) - chain, err := middlewaresBuilder.BuildChain(context.Background(), []string{"empty"}) - require.NoError(t, err) - - _, err = chain.Then(nil) + chain := middlewaresBuilder.BuildChain(context.Background(), []string{"empty"}) + _, err := chain.Then(nil) require.NoError(t, err) } @@ -125,3 +127,254 @@ func TestMiddlewaresRegistry_BuildMiddlewareAddPrefix(t *testing.T) { }) } } + +func TestChainWithContext(t *testing.T) { + testCases := []struct { + desc string + buildChain []string + configuration map[string]*config.Middleware + expected map[string]string + contextProvider string + expectedError error + }{ + { + desc: "Simple middleware", + buildChain: []string{"middleware-1"}, + configuration: map[string]*config.Middleware{ + "middleware-1": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"middleware-1": "value-middleware-1"}, + }, + }, + }, + expected: map[string]string{"middleware-1": "value-middleware-1"}, + }, + { + desc: "Middleware that references a chain", + buildChain: []string{"middleware-chain-1"}, + configuration: map[string]*config.Middleware{ + "middleware-1": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"middleware-1": "value-middleware-1"}, + }, + }, + "middleware-chain-1": { + Chain: &config.Chain{ + Middlewares: []string{"middleware-1"}, + }, + }, + }, + expected: map[string]string{"middleware-1": "value-middleware-1"}, + }, + { + desc: "Should prefix the middlewareName with the provider in the context", + buildChain: []string{"middleware-1"}, + configuration: map[string]*config.Middleware{ + "provider-1.middleware-1": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"provider-1.middleware-1": "value-middleware-1"}, + }, + }, + }, + expected: map[string]string{"provider-1.middleware-1": "value-middleware-1"}, + contextProvider: "provider-1", + }, + { + desc: "Should not prefix a qualified middlewareName with the provider in the context", + buildChain: []string{"provider-1.middleware-1"}, + configuration: map[string]*config.Middleware{ + "provider-1.middleware-1": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"provider-1.middleware-1": "value-middleware-1"}, + }, + }, + }, + expected: map[string]string{"provider-1.middleware-1": "value-middleware-1"}, + contextProvider: "provider-1", + }, + { + desc: "Should be context aware if a chain references another middleware", + buildChain: []string{"provider-1.middleware-chain-1"}, + configuration: map[string]*config.Middleware{ + "provider-1.middleware-1": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"middleware-1": "value-middleware-1"}, + }, + }, + "provider-1.middleware-chain-1": { + Chain: &config.Chain{ + Middlewares: []string{"middleware-1"}, + }, + }, + }, + expected: map[string]string{"middleware-1": "value-middleware-1"}, + }, + { + desc: "Should handle nested chains with different context", + buildChain: []string{"provider-1.middleware-chain-1", "middleware-chain-1"}, + configuration: map[string]*config.Middleware{ + "provider-1.middleware-1": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"middleware-1": "value-middleware-1"}, + }, + }, + "provider-1.middleware-2": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"middleware-2": "value-middleware-2"}, + }, + }, + "provider-1.middleware-chain-1": { + Chain: &config.Chain{ + Middlewares: []string{"middleware-1"}, + }, + }, + "provider-1.middleware-chain-2": { + Chain: &config.Chain{ + Middlewares: []string{"middleware-2"}, + }, + }, + "provider-2.middleware-chain-1": { + Chain: &config.Chain{ + Middlewares: []string{"provider-1.middleware-2", "provider-1.middleware-chain-2"}, + }, + }, + }, + expected: map[string]string{"middleware-1": "value-middleware-1", "middleware-2": "value-middleware-2"}, + contextProvider: "provider-2", + }, + { + desc: "Detects recursion in Middleware chain", + buildChain: []string{"m1"}, + configuration: map[string]*config.Middleware{ + "ok": { + Retry: &config.Retry{}, + }, + "m1": { + Chain: &config.Chain{ + Middlewares: []string{"m2"}, + }, + }, + "m2": { + Chain: &config.Chain{ + Middlewares: []string{"ok", "m3"}, + }, + }, + "m3": { + Chain: &config.Chain{ + Middlewares: []string{"m1"}, + }, + }, + }, + expectedError: errors.New("could not instantiate middleware m1: recursion detected in m1->m2->m3->m1"), + }, + { + desc: "Detects recursion in Middleware chain", + buildChain: []string{"provider.m1"}, + configuration: map[string]*config.Middleware{ + "provider2.ok": { + Retry: &config.Retry{}, + }, + "provider.m1": { + Chain: &config.Chain{ + Middlewares: []string{"provider2.m2"}, + }, + }, + "provider2.m2": { + Chain: &config.Chain{ + Middlewares: []string{"ok", "provider.m3"}, + }, + }, + "provider.m3": { + Chain: &config.Chain{ + Middlewares: []string{"m1"}, + }, + }, + }, + expectedError: errors.New("could not instantiate middleware provider.m1: recursion detected in provider.m1->provider2.m2->provider.m3->provider.m1"), + }, + { + buildChain: []string{"ok", "m0"}, + configuration: map[string]*config.Middleware{ + "ok": { + Retry: &config.Retry{}, + }, + "m0": { + Chain: &config.Chain{ + Middlewares: []string{"m0"}, + }, + }, + }, + expectedError: errors.New("could not instantiate middleware m0: recursion detected in m0->m0"), + }, + { + desc: "Detects MiddlewareChain that references a Chain that references a Chain with a missing middleware", + buildChain: []string{"m0"}, + configuration: map[string]*config.Middleware{ + "m0": { + Chain: &config.Chain{ + Middlewares: []string{"m1"}, + }, + }, + "m1": { + Chain: &config.Chain{ + Middlewares: []string{"m2"}, + }, + }, + "m2": { + Chain: &config.Chain{ + Middlewares: []string{"m3"}, + }, + }, + "m3": { + Chain: &config.Chain{ + Middlewares: []string{"m2"}, + }, + }, + }, + expectedError: errors.New("could not instantiate middleware m2: recursion detected in m0->m1->m2->m3->m2"), + }, + { + desc: "--", + buildChain: []string{"m0"}, + configuration: map[string]*config.Middleware{ + "m0": { + Chain: &config.Chain{ + Middlewares: []string{"m0"}, + }, + }, + }, + expectedError: errors.New("could not instantiate middleware m0: recursion detected in m0->m0"), + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + if len(test.contextProvider) > 0 { + ctx = internal.AddProviderInContext(ctx, test.contextProvider+".foobar") + } + + builder := NewBuilder(test.configuration, nil) + + result := builder.BuildChain(ctx, test.buildChain) + + handlers, err := result.Then(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) })) + if test.expectedError != nil { + require.NotNil(t, err) + require.Equal(t, test.expectedError.Error(), err.Error()) + } else { + require.NoError(t, err) + recorder := httptest.NewRecorder() + request, _ := http.NewRequest(http.MethodGet, "http://foo/", nil) + handlers.ServeHTTP(recorder, request) + + for key, value := range test.expected { + assert.Equal(t, value, request.Header.Get(key)) + } + } + }) + } +} diff --git a/server/router/route_appender_aggregator.go b/server/router/route_appender_aggregator.go index 0e8652969..c43b596b9 100644 --- a/server/router/route_appender_aggregator.go +++ b/server/router/route_appender_aggregator.go @@ -15,13 +15,11 @@ import ( // chainBuilder The contract of the middleware builder type chainBuilder interface { - BuildChain(ctx context.Context, middlewares []string) (*alice.Chain, error) + BuildChain(ctx context.Context, middlewares []string) *alice.Chain } // NewRouteAppenderAggregator Creates a new RouteAppenderAggregator func NewRouteAppenderAggregator(ctx context.Context, chainBuilder chainBuilder, conf static.Configuration, entryPointName string, currentConfiguration *safe.Safe) *RouteAppenderAggregator { - logger := log.FromContext(ctx) - aggregator := &RouteAppenderAggregator{} if conf.Providers != nil && conf.Providers.Rest != nil { @@ -29,46 +27,35 @@ func NewRouteAppenderAggregator(ctx context.Context, chainBuilder chainBuilder, } if conf.API != nil && conf.API.EntryPoint == entryPointName { - chain, err := chainBuilder.BuildChain(ctx, conf.API.Middlewares) - if err != nil { - logger.Error(err) - } else { - aggregator.AddAppender(&WithMiddleware{ - appender: api.Handler{ - EntryPoint: conf.API.EntryPoint, - Dashboard: conf.API.Dashboard, - Statistics: conf.API.Statistics, - DashboardAssets: conf.API.DashboardAssets, - CurrentConfigurations: currentConfiguration, - Debug: conf.Global.Debug, - }, - routerMiddlewares: chain, - }) - } + chain := chainBuilder.BuildChain(ctx, conf.API.Middlewares) + aggregator.AddAppender(&WithMiddleware{ + appender: api.Handler{ + EntryPoint: conf.API.EntryPoint, + Dashboard: conf.API.Dashboard, + Statistics: conf.API.Statistics, + DashboardAssets: conf.API.DashboardAssets, + CurrentConfigurations: currentConfiguration, + Debug: conf.Global.Debug, + }, + routerMiddlewares: chain, + }) + } if conf.Ping != nil && conf.Ping.EntryPoint == entryPointName { - chain, err := chainBuilder.BuildChain(ctx, conf.Ping.Middlewares) - if err != nil { - logger.Error(err) - } else { - aggregator.AddAppender(&WithMiddleware{ - appender: conf.Ping, - routerMiddlewares: chain, - }) - } + chain := chainBuilder.BuildChain(ctx, conf.Ping.Middlewares) + aggregator.AddAppender(&WithMiddleware{ + appender: conf.Ping, + routerMiddlewares: chain, + }) } if conf.Metrics != nil && conf.Metrics.Prometheus != nil && conf.Metrics.Prometheus.EntryPoint == entryPointName { - chain, err := chainBuilder.BuildChain(ctx, conf.Metrics.Prometheus.Middlewares) - if err != nil { - logger.Error(err) - } else { - aggregator.AddAppender(&WithMiddleware{ - appender: metrics.PrometheusHandler{}, - routerMiddlewares: chain, - }) - } + chain := chainBuilder.BuildChain(ctx, conf.Metrics.Prometheus.Middlewares) + aggregator.AddAppender(&WithMiddleware{ + appender: metrics.PrometheusHandler{}, + routerMiddlewares: chain, + }) } return aggregator diff --git a/server/router/route_appender_aggregator_test.go b/server/router/route_appender_aggregator_test.go index 65ea00f38..809058658 100644 --- a/server/router/route_appender_aggregator_test.go +++ b/server/router/route_appender_aggregator_test.go @@ -17,7 +17,7 @@ type ChainBuilderMock struct { middles map[string]alice.Constructor } -func (c *ChainBuilderMock) BuildChain(ctx context.Context, middles []string) (*alice.Chain, error) { +func (c *ChainBuilderMock) BuildChain(ctx context.Context, middles []string) *alice.Chain { chain := alice.New() for _, mName := range middles { @@ -26,7 +26,7 @@ func (c *ChainBuilderMock) BuildChain(ctx context.Context, middles []string) (*a } } - return &chain, nil + return &chain } func TestNewRouteAppenderAggregator(t *testing.T) { @@ -61,7 +61,7 @@ func TestNewRouteAppenderAggregator(t *testing.T) { expected: map[string]int{ "/wrong": http.StatusBadGateway, "/ping": http.StatusOK, - //"/.well-known/acme-challenge/token": http.StatusNotFound, // FIXME + // "/.well-known/acme-challenge/token": http.StatusNotFound, // FIXME "/api/providers": http.StatusUnauthorized, }, }, diff --git a/server/router/router.go b/server/router/router.go index 2fd958d9f..76bedb906 100644 --- a/server/router/router.go +++ b/server/router/router.go @@ -13,6 +13,7 @@ import ( "github.com/containous/traefik/middlewares/recovery" "github.com/containous/traefik/middlewares/tracing" "github.com/containous/traefik/responsemodifiers" + "github.com/containous/traefik/server/internal" "github.com/containous/traefik/server/middleware" "github.com/containous/traefik/server/service" ) @@ -104,9 +105,11 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string SkipClean(true) for routerName, routerConfig := range configs { - ctx = log.With(ctx, log.Str(log.RouterName, routerName)) + ctx := log.With(ctx, log.Str(log.RouterName, routerName)) logger := log.FromContext(ctx) + ctx = internal.AddProviderInContext(ctx, routerName) + handler, err := m.buildRouterHandler(ctx, routerName) if err != nil { logger.Error(err) @@ -166,10 +169,7 @@ func (m *Manager) buildHandler(ctx context.Context, router *config.Router, route return nil, err } - mHandler, err := m.middlewaresBuilder.BuildChain(ctx, router.Middlewares) - if err != nil { - return nil, err - } + mHandler := m.middlewaresBuilder.BuildChain(ctx, router.Middlewares) alHandler := func(next http.Handler) (http.Handler, error) { return accesslog.NewFieldHandler(next, accesslog.ServiceName, router.Service, accesslog.AddServiceFields), nil diff --git a/server/router/router_test.go b/server/router/router_test.go index b133faacf..cc7944452 100644 --- a/server/router/router_test.go +++ b/server/router/router_test.go @@ -209,6 +209,102 @@ func TestRouterManager_Get(t *testing.T) { }, }, }, + { + desc: "no middleware with provider name", + routersConfig: map[string]*config.Router{ + "provider-1.foo": { + EntryPoints: []string{"web"}, + Service: "foo-service", + Rule: "Host:foo.bar", + }, + }, + serviceConfig: map[string]*config.Service{ + "provider-1.foo-service": { + LoadBalancer: &config.LoadBalancerService{ + Servers: []config.Server{ + { + URL: server.URL, + Weight: 1, + }, + }, + Method: "wrr", + }, + }, + }, + entryPoints: []string{"web"}, + expected: ExpectedResult{StatusCode: http.StatusOK}, + }, + { + desc: "no middleware with specified provider name", + routersConfig: map[string]*config.Router{ + "provider-1.foo": { + EntryPoints: []string{"web"}, + Service: "provider-2.foo-service", + Rule: "Host:foo.bar", + }, + }, + serviceConfig: map[string]*config.Service{ + "provider-2.foo-service": { + LoadBalancer: &config.LoadBalancerService{ + Servers: []config.Server{ + { + URL: server.URL, + Weight: 1, + }, + }, + Method: "wrr", + }, + }, + }, + entryPoints: []string{"web"}, + expected: ExpectedResult{StatusCode: http.StatusOK}, + }, + { + desc: "middleware: chain with provider name", + routersConfig: map[string]*config.Router{ + "provider-1.foo": { + EntryPoints: []string{"web"}, + Middlewares: []string{"provider-2.chain-middle", "headers-middle"}, + Service: "foo-service", + Rule: "Host:foo.bar", + }, + }, + serviceConfig: map[string]*config.Service{ + "provider-1.foo-service": { + LoadBalancer: &config.LoadBalancerService{ + Servers: []config.Server{ + { + URL: server.URL, + Weight: 1, + }, + }, + Method: "wrr", + }, + }, + }, + middlewaresConfig: map[string]*config.Middleware{ + "provider-2.chain-middle": { + Chain: &config.Chain{Middlewares: []string{"auth-middle"}}, + }, + "provider-2.auth-middle": { + BasicAuth: &config.BasicAuth{ + Users: []string{"toto:titi"}, + }, + }, + "provider-1.headers-middle": { + Headers: &config.Headers{ + CustomRequestHeaders: map[string]string{"X-Apero": "beer"}, + }, + }, + }, + entryPoints: []string{"web"}, + expected: ExpectedResult{ + StatusCode: http.StatusUnauthorized, + RequestHeaders: map[string]string{ + "X-Apero": "", + }, + }, + }, } for _, test := range testCases { diff --git a/server/server_configuration.go b/server/server_configuration.go index bebe64c11..4a1122550 100644 --- a/server/server_configuration.go +++ b/server/server_configuration.go @@ -76,28 +76,7 @@ func (s *Server) loadConfig(configurations config.Configurations) (map[string]ht ctx := context.TODO() - // FIXME manage duplicates - conf := config.Configuration{ - Routers: make(map[string]*config.Router), - Middlewares: make(map[string]*config.Middleware), - Services: make(map[string]*config.Service), - } - for _, config := range configurations { - for key, value := range config.Middlewares { - conf.Middlewares[key] = value - } - - for key, value := range config.Services { - conf.Services[key] = value - } - - for key, value := range config.Routers { - conf.Routers[key] = value - } - - conf.TLS = append(conf.TLS, config.TLS...) - } - + conf := mergeConfiguration(configurations) handlers := s.applyConfiguration(ctx, conf) // Get new certificates list sorted per entry points diff --git a/server/service/service.go b/server/service/service.go index cfbb45c8d..3640a623b 100644 --- a/server/service/service.go +++ b/server/service/service.go @@ -16,6 +16,7 @@ import ( "github.com/containous/traefik/middlewares/emptybackendhandler" "github.com/containous/traefik/old/middlewares/pipelining" "github.com/containous/traefik/server/cookie" + "github.com/containous/traefik/server/internal" "github.com/vulcand/oxy/forward" "github.com/vulcand/oxy/roundrobin" ) @@ -58,9 +59,11 @@ type Manager struct { func (m *Manager) Build(rootCtx context.Context, serviceName string, responseModifier func(*http.Response) error) (http.Handler, error) { ctx := log.With(rootCtx, log.Str(log.ServiceName, serviceName)) - // TODO refactor ? + serviceName = internal.GetQualifiedName(ctx, serviceName) + ctx = internal.AddProviderInContext(ctx, serviceName) + if conf, ok := m.configs[serviceName]; ok { - // FIXME Should handle multiple service types + // TODO Should handle multiple service types if conf.LoadBalancer != nil { return m.getLoadBalancerServiceHandler(ctx, serviceName, conf.LoadBalancer, responseModifier) } diff --git a/server/service/service_test.go b/server/service/service_test.go index 5bcb1d3b5..4873bb87f 100644 --- a/server/service/service_test.go +++ b/server/service/service_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/containous/traefik/config" + "github.com/containous/traefik/server/internal" "github.com/containous/traefik/testhelpers" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -324,4 +325,59 @@ func TestGetLoadBalancerServiceHandler(t *testing.T) { } } +func TestManager_Build(t *testing.T) { + testCases := []struct { + desc string + serviceName string + configs map[string]*config.Service + providerName string + }{ + { + desc: "Simple service name", + serviceName: "serviceName", + configs: map[string]*config.Service{ + "serviceName": { + LoadBalancer: &config.LoadBalancerService{Method: "wrr"}, + }, + }, + }, + { + desc: "Service name with provider", + serviceName: "provider-1.serviceName", + configs: map[string]*config.Service{ + "provider-1.serviceName": { + LoadBalancer: &config.LoadBalancerService{Method: "wrr"}, + }, + }, + }, + { + desc: "Service name with provider in context", + serviceName: "serviceName", + configs: map[string]*config.Service{ + "provider-1.serviceName": { + LoadBalancer: &config.LoadBalancerService{Method: "wrr"}, + }, + }, + providerName: "provider-1", + }, + } + + for _, test := range testCases { + test := test + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + manager := NewManager(test.configs, http.DefaultTransport) + + ctx := context.Background() + if len(test.providerName) > 0 { + ctx = internal.AddProviderInContext(ctx, test.providerName+".foobar") + } + + _, err := manager.Build(ctx, test.serviceName, nil) + require.NoError(t, err) + }) + } +} + // FIXME Add healthcheck tests