TLS configuration / TLS options conflict handling

diff --git a/integration/https_test.go b/integration/https_test.go index 66560a3a80..2a9075c17a 100644 --- a/integration/https_test.go +++ b/integration/https_test.go @@ -315,7 +315,7 @@ func (s *HTTPSSuite) TestWithConflictingTLSOptions() { assert.ErrorContains(s.T(), err, "tls: no supported versions satisfy MinVersion and MaxVersion") // with unknown tls option - err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("found different TLS options for routers on the same host, so using the default TLS options instead")) + err = try.GetRequest("http://127.0.0.1:8080/api/rawdata", 1*time.Second, try.BodyContains("router's TLSOptions configuration is conflicting with other routers on the same entrypoint and host, default TLS options will be used instead")) require.NoError(s.T(), err) } diff --git a/integration/simple_test.go b/integration/simple_test.go index 0d0423560c..10cdbeaaf8 100644 --- a/integration/simple_test.go +++ b/integration/simple_test.go @@ -667,7 +667,7 @@ func (s *SimpleSuite) TestRouterConfigErrors() { s.traefikCmd(withConfigFile(file)) // All errors - err := try.GetRequest("http://127.0.0.1:8080/api/http/routers", 1000*time.Millisecond, try.BodyContains(`["middleware \"unknown@file\" does not exist","found different TLS options for routers on the same host, so using the default TLS options instead"]`)) + err := try.GetRequest("http://127.0.0.1:8080/api/http/routers", 1000*time.Millisecond, try.BodyContains(`["middleware \"unknown@file\" does not exist","router's TLSOptions configuration is conflicting with other routers on the same entrypoint and host, default TLS options will be used instead"]`)) require.NoError(s.T(), err) // router3 has an error because it uses an unknown entrypoint @@ -675,11 +675,11 @@ func (s *SimpleSuite) TestRouterConfigErrors() { require.NoError(s.T(), err) // router4 is enabled, but in warning state because its tls options conf was messed up - err = try.GetRequest("http://127.0.0.1:8080/api/http/routers/websecure-router4@file", 1000*time.Millisecond, try.BodyContains(`"status":"warning"`)) + err = try.GetRequest("http://127.0.0.1:8080/api/http/routers/websecure-conflicted-router4@file", 1000*time.Millisecond, try.BodyContains(`"status":"warning"`)) require.NoError(s.T(), err) // router5 is disabled because its middleware conf is broken - err = try.GetRequest("http://127.0.0.1:8080/api/http/routers/websecure-router5@file", 1000*time.Millisecond, try.BodyContains()) + err = try.GetRequest("http://127.0.0.1:8080/api/http/routers/websecure-conflicted-router5@file", 1000*time.Millisecond, try.BodyContains()) require.NoError(s.T(), err) } diff --git a/pkg/server/aggregator.go b/pkg/server/aggregator.go index 9397710411..ba88f8bfc5 100644 --- a/pkg/server/aggregator.go +++ b/pkg/server/aggregator.go @@ -150,9 +150,8 @@ func mergeConfiguration(configurations dynamic.Configurations, defaultEntryPoint // // A router keeps its original name, and its resolved TLS options, for the entryPoints // on which it does not conflict. For each entryPoint on which it conflicts, that -// entryPoint is removed from the router and a dedicated copy is emitted, prefixed with -// the entryPoint name the same way applyModel does (ep-name), with its TLS options reset -// to the default ones. +// entryPoint is removed from the router and a dedicated copy is emitted, with its +// TLSOptions reset to the default one, named following the "ep-conflicted-name@provider" pattern. func resolveHTTPTLSOptions(routers map[string]*dynamic.Router) map[string]*dynamic.Router { if len(routers) == 0 { return routers @@ -187,7 +186,7 @@ func resolveHTTPTLSOptions(routers map[string]*dynamic.Router) map[string]*dynam // Resolve the TLS options independently for each entryPoint. conflictingRouters := make(map[string][]string, len(routersByEntryPoint)) for ep, epRouters := range routersByEntryPoint { - conflictingRouters[ep] = findConflictingRouters(epRouters) + conflictingRouters[ep] = findConflictingRouters(ep, epRouters) } for name, router := range routers { @@ -197,7 +196,9 @@ func resolveHTTPTLSOptions(routers map[string]*dynamic.Router) map[string]*dynam rt := router.DeepCopy() rt.TLS.ResolvedOptions = traefiktls.DefaultTLSConfigName rt.EntryPoints = []string{ep} - newRouters[ep+"-"+name] = rt + // The new name is not collision free but has very small possibility to collide. + // TODO: rework this naming whenever we'll introduce a resource reference mechanism not based on a string. + newRouters[ep+"-conflicted-"+name] = rt } return deleted @@ -215,7 +216,7 @@ func resolveHTTPTLSOptions(routers map[string]*dynamic.Router) map[string]*dynam // single-entryPoint routers, that serve a host (SNI) also served by another router // with a different resolved TLS option. Such routers are arbitrated by falling back // to the default TLS options. -func findConflictingRouters(routers map[string]*dynamic.Router) []string { +func findConflictingRouters(ep string, routers map[string]*dynamic.Router) []string { var conflicting []string // For each host (SNI, already lower-cased by the domain parsing), the routers @@ -233,9 +234,9 @@ func findConflictingRouters(routers map[string]*dynamic.Router) []string { continue } - // A router without a domain in its rule cannot be matched against an SNI, - // so it always falls back to the default TLS options. - if len(domains) == 0 { + // The configured TLSOptions on a router without a domain in its rule cannot be selected when evaluating the SNI, + // so if it is not the default one, it is a conflict. + if len(domains) == 0 && router.TLS.ResolvedOptions != traefiktls.DefaultTLSConfigName { conflicting = append(conflicting, name) continue } @@ -249,14 +250,18 @@ func findConflictingRouters(routers map[string]*dynamic.Router) []string { } } - for _, routersByOption := range routersByHostAndOption { + for domain, routersByOption := range routersByHostAndOption { if len(routersByOption) == 1 { continue } + var routersInConflict []string for _, names := range routersByOption { conflicting = append(conflicting, names...) + routersInConflict = append(routersInConflict, names...) } + + log.WithoutContext().Errorf("On EntryPoint %q, Host %q is served by multiple routers with different TLS options, default TLSOptions will be applied for the following routers: %v", ep, domain, routersInConflict) } return conflicting diff --git a/pkg/server/aggregator_test.go b/pkg/server/aggregator_test.go index c0c24e1712..02ee3495cd 100644 --- a/pkg/server/aggregator_test.go +++ b/pkg/server/aggregator_test.go @@ -693,8 +693,8 @@ func Test_resolveHTTPTLSOptions(t *testing.T) { "router-b@file": {EntryPoints: []string{"ep-a"}, Rule: "Host(`example.com`)", TLS: &dynamic.RouterTLSConfig{Options: "optsB"}}, }, expected: map[string]string{ - "ep-a-router-a@file": "default", - "ep-a-router-b@file": "default", + "ep-a-conflicted-router-a@file": "default", + "ep-a-conflicted-router-b@file": "default", }, unexpectedRouters: []string{"router-a@file", "router-b@file"}, }, @@ -716,22 +716,44 @@ func Test_resolveHTTPTLSOptions(t *testing.T) { "other@file": {EntryPoints: []string{"ep-a"}, Rule: "Host(`example.com`)", TLS: &dynamic.RouterTLSConfig{Options: "optsY"}}, }, expected: map[string]string{ - "ep-a-shared@file": "default", // conflicts with other@file on ep-a - "shared@file": "optsX@file", // alone on ep-b - "ep-a-other@file": "default", + "ep-a-conflicted-shared@file": "default", // conflicts with other@file on ep-a + "shared@file": "optsX@file", // alone on ep-b + "ep-a-conflicted-other@file": "default", }, unexpectedRouters: []string{"other@file"}, }, { - desc: "no domain in rule: depends on SNI, resolves to default", + desc: "no domain in rule, non-default options: forced to default and renamed", routers: map[string]*dynamic.Router{ "router-a@file": {EntryPoints: []string{"ep-a"}, Rule: "PathPrefix(`/foo`)", TLS: &dynamic.RouterTLSConfig{Options: "optsA"}}, }, expected: map[string]string{ - "ep-a-router-a@file": "default", + "ep-a-conflicted-router-a@file": "default", }, unexpectedRouters: []string{"router-a@file"}, }, + { + desc: "no domain in rule, implicit default options: not conflicting, keeps its name", + routers: map[string]*dynamic.Router{ + "router-a@file": {EntryPoints: []string{"ep-a"}, Rule: "PathPrefix(`/foo`)", TLS: &dynamic.RouterTLSConfig{}}, + }, + expected: map[string]string{ + "router-a@file": "default", + }, + unexpectedRouters: []string{"ep-a-conflicted-router-a@file"}, + }, + { + desc: "no domain in rule, explicit default options: not conflicting, keeps its name", + routers: map[string]*dynamic.Router{ + "router-a@file": {EntryPoints: []string{"ep-a"}, Rule: "PathPrefix(`/foo`)", TLS: &dynamic.RouterTLSConfig{ + Options: "default", + }}, + }, + expected: map[string]string{ + "router-a@file": "default", + }, + unexpectedRouters: []string{"ep-a-conflicted-router-a@file"}, + }, } for _, test := range testCases { diff --git a/pkg/server/router/tcp/manager.go b/pkg/server/router/tcp/manager.go index de9e9e2762..db21ab9f10 100644 --- a/pkg/server/router/tcp/manager.go +++ b/pkg/server/router/tcp/manager.go @@ -167,8 +167,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string } if len(domains) > 0 && routerHTTPConfig.TLS.ResolvedOptions != tlsOptionsName { - logger.Warn("Found different TLS options for routers on the same host, so using the default TLS options instead.") - routerHTTPConfig.AddError(errors.New("found different TLS options for routers on the same host, so using the default TLS options instead"), false) + routerHTTPConfig.AddError(errors.New("router's TLSOptions configuration is conflicting with other routers on the same entrypoint and host, default TLS options will be used instead"), false) } // Even though the error is seemingly ignored (aside from logging it),