Merge branch v2.4 into master

pkg/metrics/pilot_test.go

@@ -300,6 +300,8 @@ func findPilotMetric(name string, metrics []PilotMetric) *PilotMetric {
 }
 
 func buildPilotCounterAssert(t *testing.T, metricName string, expectedValue float64) func(metric *PilotMetric) {
+	t.Helper()
+
 	return func(metric *PilotMetric) {
 		for _, value := range metric.Observations {
 			if cv := value.(float64); cv != expectedValue {
@@ -311,6 +313,8 @@ func buildPilotCounterAssert(t *testing.T, metricName string, expectedValue floa
 }
 
 func buildPilotGreaterThanCounterAssert(t *testing.T, metricName string, expectedMinValue float64) func(metric *PilotMetric) {
+	t.Helper()
+
 	return func(metric *PilotMetric) {
 		for _, value := range metric.Observations {
 			if cv := value.(float64); cv < expectedMinValue {
@@ -322,6 +326,8 @@ func buildPilotGreaterThanCounterAssert(t *testing.T, metricName string, expecte
 }
 
 func buildPilotHistogramAssert(t *testing.T, metricName string, expectedSampleCount float64) func(metric *PilotMetric) {
+	t.Helper()
+
 	return func(metric *PilotMetric) {
 		for _, value := range metric.Observations {
 			if pho := value.(*pilotHistogramObservation); pho.Count != expectedSampleCount {
@@ -333,6 +339,8 @@ func buildPilotHistogramAssert(t *testing.T, metricName string, expectedSampleCo
 }
 
 func buildPilotGaugeAssert(t *testing.T, metricName string, expectedValue float64) func(metric *PilotMetric) {
+	t.Helper()
+
 	return func(metric *PilotMetric) {
 		for _, value := range metric.Observations {
 			if gv := value.(float64); gv != expectedValue {
@@ -344,6 +352,8 @@ func buildPilotGaugeAssert(t *testing.T, metricName string, expectedValue float6
 }
 
 func buildPilotTimestampAssert(t *testing.T, metricName string) func(metric *PilotMetric) {
+	t.Helper()
+
 	return func(metric *PilotMetric) {
 		for _, value := range metric.Observations {
 			if ts := time.Unix(int64(value.(float64)), 0); time.Since(ts) > time.Minute {

pkg/metrics/prometheus_test.go

@@ -486,6 +486,8 @@ func assertCounterValue(t *testing.T, want float64, family *dto.MetricFamily, la
 }
 
 func buildCounterAssert(t *testing.T, metricName string, expectedValue int) func(family *dto.MetricFamily) {
+	t.Helper()
+
 	return func(family *dto.MetricFamily) {
 		if cv := int(family.Metric[0].Counter.GetValue()); cv != expectedValue {
 			t.Errorf("metric %s has value %d, want %d", metricName, cv, expectedValue)
@@ -494,6 +496,8 @@ func buildCounterAssert(t *testing.T, metricName string, expectedValue int) func
 }
 
 func buildGreaterThanCounterAssert(t *testing.T, metricName string, expectedMinValue int) func(family *dto.MetricFamily) {
+	t.Helper()
+
 	return func(family *dto.MetricFamily) {
 		if cv := int(family.Metric[0].Counter.GetValue()); cv < expectedMinValue {
 			t.Errorf("metric %s has value %d, want at least %d", metricName, cv, expectedMinValue)
@@ -502,6 +506,8 @@ func buildGreaterThanCounterAssert(t *testing.T, metricName string, expectedMinV
 }
 
 func buildHistogramAssert(t *testing.T, metricName string, expectedSampleCount int) func(family *dto.MetricFamily) {
+	t.Helper()
+
 	return func(family *dto.MetricFamily) {
 		if sc := int(family.Metric[0].Histogram.GetSampleCount()); sc != expectedSampleCount {
 			t.Errorf("metric %s has sample count value %d, want %d", metricName, sc, expectedSampleCount)
@@ -510,6 +516,8 @@ func buildHistogramAssert(t *testing.T, metricName string, expectedSampleCount i
 }
 
 func buildGaugeAssert(t *testing.T, metricName string, expectedValue int) func(family *dto.MetricFamily) {
+	t.Helper()
+
 	return func(family *dto.MetricFamily) {
 		if gv := int(family.Metric[0].Gauge.GetValue()); gv != expectedValue {
 			t.Errorf("metric %s has value %d, want %d", metricName, gv, expectedValue)
@@ -518,6 +526,8 @@ func buildGaugeAssert(t *testing.T, metricName string, expectedValue int) func(f
 }
 
 func buildTimestampAssert(t *testing.T, metricName string) func(family *dto.MetricFamily) {
+	t.Helper()
+
 	return func(family *dto.MetricFamily) {
 		if ts := time.Unix(int64(family.Metric[0].Gauge.GetValue()), 0); time.Since(ts) > time.Minute {
 			t.Errorf("metric %s has wrong timestamp %v", metricName, ts)

pkg/middlewares/accesslog/logger_test.go

@@ -683,6 +683,8 @@ func TestNewLogHandlerOutputStdout(t *testing.T) {
 }
 
 func assertValidLogData(t *testing.T, expected string, logData []byte) {
+	t.Helper()
+
 	if len(expected) == 0 {
 		assert.Zero(t, len(logData))
 		t.Log(string(logData))
@@ -716,6 +718,8 @@ func assertValidLogData(t *testing.T, expected string, logData []byte) {
 }
 
 func captureStdout(t *testing.T) (out *os.File, restoreStdout func()) {
+	t.Helper()
+
 	file, err := ioutil.TempFile("", "testlogger")
 	require.NoError(t, err, "failed to create temp file")
 
@@ -731,6 +735,8 @@ func captureStdout(t *testing.T) (out *os.File, restoreStdout func()) {
 }
 
 func createTempDir(t *testing.T, prefix string) string {
+	t.Helper()
+
 	tmpDir, err := ioutil.TempDir("", prefix)
 	require.NoError(t, err, "failed to create temp dir")
 
@@ -740,6 +746,8 @@ func createTempDir(t *testing.T, prefix string) string {
 }
 
 func doLoggingTLSOpt(t *testing.T, config *types.AccessLog, enableTLS bool) {
+	t.Helper()
+
 	logger, err := NewHandler(config)
 	require.NoError(t, err)
 	defer logger.Close()
@@ -771,10 +779,14 @@ func doLoggingTLSOpt(t *testing.T, config *types.AccessLog, enableTLS bool) {
 }
 
 func doLoggingTLS(t *testing.T, config *types.AccessLog) {
+	t.Helper()
+
 	doLoggingTLSOpt(t, config, true)
 }
 
 func doLogging(t *testing.T, config *types.AccessLog) {
+	t.Helper()
+
 	doLoggingTLSOpt(t, config, false)
 }
 

pkg/provider/file/file_test.go

@@ -76,8 +76,7 @@ func TestErrorWhenEmptyConfig(t *testing.T) {
 func TestProvideWithoutWatch(t *testing.T) {
 	for _, test := range getTestCases() {
 		t.Run(test.desc+" without watch", func(t *testing.T) {
-			provider, clean := createProvider(t, test, false)
-			defer clean()
+			provider := createProvider(t, test, false)
 			configChan := make(chan dynamic.Message)
 
 			provider.DebugLogGeneratedTemplate = true
@@ -109,8 +108,7 @@ func TestProvideWithoutWatch(t *testing.T) {
 func TestProvideWithWatch(t *testing.T) {
 	for _, test := range getTestCases() {
 		t.Run(test.desc+" with watch", func(t *testing.T) {
-			provider, clean := createProvider(t, test, true)
-			defer clean()
+			provider := createProvider(t, test, true)
 			configChan := make(chan dynamic.Message)
 
 			go func() {
@@ -244,7 +242,9 @@ func getTestCases() []ProvideTestCase {
 	}
 }
 
-func createProvider(t *testing.T, test ProvideTestCase, watch bool) (*Provider, func()) {
+func createProvider(t *testing.T, test ProvideTestCase, watch bool) *Provider {
+	t.Helper()
+
 	tempDir := createTempDir(t, "testdir")
 
 	provider := &Provider{}
@@ -276,9 +276,11 @@ func createProvider(t *testing.T, test ProvideTestCase, watch bool) (*Provider,
 		provider.Filename = file.Name()
 	}
 
-	return provider, func() {
+	t.Cleanup(func() {
 		os.RemoveAll(tempDir)
-	}
+	})
+
+	return provider
 }
 
 // createTempDir Helper.

pkg/provider/kubernetes/ingress/fixtures/Ingress-with-conflicting-routers-on-host_endpoint.yml

@@ -0,0 +1,15 @@
+kind: Endpoints
+apiVersion: v1
+metadata:
+  name: service1
+  namespace: testing
+
+subsets:
+- addresses:
+  - ip: 10.10.0.1
+  ports:
+  - port: 8080
+- addresses:
+  - ip: 10.21.0.1
+  ports:
+  - port: 8080

pkg/provider/kubernetes/ingress/fixtures/Ingress-with-conflicting-routers-on-host_ingress.yml

@@ -0,0 +1,23 @@
+kind: Ingress
+apiVersion: networking.k8s.io/v1beta1
+metadata:
+  name: ""
+  namespace: testing
+
+spec:
+  rules:
+  - host: "*.bar"
+    http:
+      paths:
+      - path: /bar
+        backend:
+          serviceName: service1
+          servicePort: 80
+
+  - host: "bar"
+    http:
+      paths:
+      - path: /bar
+        backend:
+          serviceName: service1
+          servicePort: 80

pkg/provider/kubernetes/ingress/fixtures/Ingress-with-conflicting-routers-on-host_service.yml

@@ -0,0 +1,10 @@
+kind: Service
+apiVersion: v1
+metadata:
+  name: service1
+  namespace: testing
+
+spec:
+  ports:
+  - port: 80
+  clusterIp: 10.0.0.1

pkg/provider/kubernetes/ingress/fixtures/Ingress-with-conflicting-routers-on-path_endpoint.yml

@@ -0,0 +1,15 @@
+kind: Endpoints
+apiVersion: v1
+metadata:
+  name: service1
+  namespace: testing
+
+subsets:
+- addresses:
+  - ip: 10.10.0.1
+  ports:
+  - port: 8080
+- addresses:
+  - ip: 10.21.0.1
+  ports:
+  - port: 8080

pkg/provider/kubernetes/ingress/fixtures/Ingress-with-conflicting-routers-on-path_ingress.yml

@@ -0,0 +1,19 @@
+kind: Ingress
+apiVersion: networking.k8s.io/v1beta1
+metadata:
+  name: ""
+  namespace: testing
+
+spec:
+  rules:
+  - http:
+      paths:
+      - path: /foo/bar
+        backend:
+          serviceName: service1
+          servicePort: 80
+
+      - path: /foo-bar
+        backend:
+          serviceName: service1
+          servicePort: 80

pkg/provider/kubernetes/ingress/fixtures/Ingress-with-conflicting-routers-on-path_service.yml

@@ -0,0 +1,10 @@
+kind: Service
+apiVersion: v1
+metadata:
+  name: service1
+  namespace: testing
+
+spec:
+  ports:
+  - port: 80
+  clusterIp: 10.0.0.1

pkg/provider/kubernetes/ingress/kubernetes.go

@@ -2,6 +2,7 @@ package ingress
 
 import (
 	"context"
+	"crypto/sha256"
 	"errors"
 	"fmt"
 	"math"
@@ -252,6 +253,8 @@ func (p *Provider) loadConfigurationFromIngresses(ctx context.Context, client Cl
 			conf.HTTP.Services["default-backend"] = service
 		}
 
+		routers := map[string][]*dynamic.Router{}
+
 		for _, rule := range ingress.Spec.Rules {
 			if err := p.updateIngressStatus(ingress, client); err != nil {
 				log.FromContext(ctx).Errorf("Error while updating ingress status: %v", err)
@@ -275,8 +278,26 @@ func (p *Provider) loadConfigurationFromIngresses(ctx context.Context, client Cl
 				conf.HTTP.Services[serviceName] = service
 
 				routerKey := strings.TrimPrefix(provider.Normalize(ingress.Name+"-"+ingress.Namespace+"-"+rule.Host+pa.Path), "-")
+				routers[routerKey] = append(routers[routerKey], loadRouter(rule, pa, rtConfig, serviceName))
+			}
+		}
 
-				conf.HTTP.Routers[routerKey] = loadRouter(rule, pa, rtConfig, serviceName)
+		for routerKey, conflictingRouters := range routers {
+			if len(conflictingRouters) == 1 {
+				conf.HTTP.Routers[routerKey] = conflictingRouters[0]
+				continue
+			}
+
+			log.FromContext(ctx).Debugf("Multiple routers are defined with the same key %q, generating hashes to avoid conflicts", routerKey)
+
+			for _, router := range conflictingRouters {
+				key, err := makeRouterKeyWithHash(routerKey, router.Rule)
+				if err != nil {
+					log.FromContext(ctx).Error(err)
+					continue
+				}
+
+				conf.HTTP.Routers[key] = router
 			}
 		}
 	}
@@ -542,6 +563,17 @@ func getProtocol(portSpec corev1.ServicePort, portName string, svcConfig *Servic
 	return protocol
 }
 
+func makeRouterKeyWithHash(key, rule string) (string, error) {
+	h := sha256.New()
+	if _, err := h.Write([]byte(rule)); err != nil {
+		return "", err
+	}
+
+	dupKey := fmt.Sprintf("%s-%.10x", key, h.Sum(nil))
+
+	return dupKey, nil
+}
+
 func loadRouter(rule networkingv1beta1.IngressRule, pa networkingv1beta1.HTTPIngressPath, rtConfig *RouterConfig, serviceName string) *dynamic.Router {
 	var rules []string
 	if len(rule.Host) > 0 {

pkg/provider/kubernetes/ingress/kubernetes_test.go

@@ -169,6 +169,74 @@ func TestLoadConfigurationFromIngresses(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "Ingress with conflicting routers on host",
+			expected: &dynamic.Configuration{
+				TCP: &dynamic.TCPConfiguration{},
+				HTTP: &dynamic.HTTPConfiguration{
+					Middlewares: map[string]*dynamic.Middleware{},
+					Routers: map[string]*dynamic.Router{
+						"testing-bar-bar-3be6cfd7daba66cf2fdd": {
+							Rule:    "HostRegexp(`{subdomain:[a-zA-Z0-9-]+}.bar`) && PathPrefix(`/bar`)",
+							Service: "testing-service1-80",
+						},
+						"testing-bar-bar-636bf36c00fedaab3d44": {
+							Rule:    "Host(`bar`) && PathPrefix(`/bar`)",
+							Service: "testing-service1-80",
+						},
+					},
+					Services: map[string]*dynamic.Service{
+						"testing-service1-80": {
+							LoadBalancer: &dynamic.ServersLoadBalancer{
+								PassHostHeader: Bool(true),
+								Servers: []dynamic.Server{
+									{
+										URL: "http://10.10.0.1:8080",
+									},
+									{
+										URL: "http://10.21.0.1:8080",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			desc: "Ingress with conflicting routers on path",
+			expected: &dynamic.Configuration{
+				TCP: &dynamic.TCPConfiguration{},
+				HTTP: &dynamic.HTTPConfiguration{
+					Middlewares: map[string]*dynamic.Middleware{},
+					Routers: map[string]*dynamic.Router{
+						"testing-foo-bar-d0b30949e54d6a7515ca": {
+							Rule:    "PathPrefix(`/foo/bar`)",
+							Service: "testing-service1-80",
+						},
+						"testing-foo-bar-dcd54bae39a6d7557f48": {
+							Rule:    "PathPrefix(`/foo-bar`)",
+							Service: "testing-service1-80",
+						},
+					},
+					Services: map[string]*dynamic.Service{
+						"testing-service1-80": {
+							LoadBalancer: &dynamic.ServersLoadBalancer{
+								PassHostHeader: Bool(true),
+								Servers: []dynamic.Server{
+									{
+										URL: "http://10.10.0.1:8080",
+									},
+									{
+										URL: "http://10.21.0.1:8080",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 		{
 			desc: "Ingress one rule with two paths",
 			expected: &dynamic.Configuration{

pkg/rules/rules.go

@@ -59,7 +59,14 @@ func (r *Router) AddRoute(rule string, priority int, handler http.Handler) error
 	}
 
 	route := r.NewRoute().Handler(handler).Priority(priority)
-	return addRuleOnRoute(route, buildTree())
+
+	err = addRuleOnRoute(route, buildTree())
+	if err != nil {
+		route.BuildOnly()
+		return err
+	}
+
+	return nil
 }
 
 type tree struct {

pkg/rules/rules_test.go

@@ -29,6 +29,16 @@ func Test_addRoute(t *testing.T) {
 			rule:          "rulewithnotmatcher",
 			expectedError: true,
 		},
+		{
+			desc:          "Host empty",
+			rule:          "Host(``)",
+			expectedError: true,
+		},
+		{
+			desc:          "PathPrefix empty",
+			rule:          "PathPrefix(``)",
+			expectedError: true,
+		},
 		{
 			desc: "PathPrefix",
 			rule: "PathPrefix(`/foo`)",

pkg/server/router/router_test.go

@@ -62,6 +62,29 @@ func TestRouterManager_Get(t *testing.T) {
 			entryPoints: []string{"web"},
 			expected:    expectedResult{StatusCode: http.StatusOK},
 		},
+		{
+			desc: "empty host",
+			routersConfig: map[string]*dynamic.Router{
+				"foo": {
+					EntryPoints: []string{"web"},
+					Service:     "foo-service",
+					Rule:        "Host(``)",
+				},
+			},
+			serviceConfig: map[string]*dynamic.Service{
+				"foo-service": {
+					LoadBalancer: &dynamic.ServersLoadBalancer{
+						Servers: []dynamic.Server{
+							{
+								URL: server.URL,
+							},
+						},
+					},
+				},
+			},
+			entryPoints: []string{"web"},
+			expected:    expectedResult{StatusCode: http.StatusNotFound},
+		},
 		{
 			desc: "no load balancer",
 			routersConfig: map[string]*dynamic.Router{

pkg/server/server_entrypoint_tcp_test.go

@@ -28,6 +28,7 @@ func TestShutdownHijacked(t *testing.T) {
 		err = resp.Write(conn)
 		require.NoError(t, err)
 	}))
+
 	testShutdown(t, router)
 }
 
@@ -37,6 +38,7 @@ func TestShutdownHTTP(t *testing.T) {
 		rw.WriteHeader(http.StatusOK)
 		time.Sleep(time.Second)
 	}))
+
 	testShutdown(t, router)
 }
 
@@ -61,6 +63,8 @@ func TestShutdownTCP(t *testing.T) {
 }
 
 func testShutdown(t *testing.T, router *tcp.Router) {
+	t.Helper()
+
 	epConfig := &static.EntryPointsTransport{}
 	epConfig.SetDefaults()
 

pkg/server/server_entrypoint_udp_test.go

@@ -103,6 +103,8 @@ func TestShutdownUDPConn(t *testing.T) {
 // It fatals if the read blocks longer than timeout, which is useful to detect
 // regressions that would make a test wait forever.
 func requireEcho(t *testing.T, data string, conn io.ReadWriter, timeout time.Duration) {
+	t.Helper()
+
 	_, err := conn.Write([]byte(data))
 	require.NoError(t, err)
 

pkg/server/service/proxy_websocket_test.go

@@ -696,12 +696,16 @@ func (w *websocketRequest) open() (*websocket.Conn, net.Conn, error) {
 }
 
 func parseURI(t *testing.T, uri string) *url.URL {
+	t.Helper()
+
 	out, err := url.ParseRequestURI(uri)
 	require.NoError(t, err)
 	return out
 }
 
 func createProxyWithForwarder(t *testing.T, proxy http.Handler, url string) *httptest.Server {
+	t.Helper()
+
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
 		path := req.URL.Path // keep the original path
 		// Set new backend URL

pkg/tcp/proxy_test.go

@@ -17,6 +17,8 @@ import (
 )
 
 func fakeRedis(t *testing.T, listener net.Listener) {
+	t.Helper()
+
 	for {
 		conn, err := listener.Accept()
 		fmt.Println("Accept on server")

pkg/udp/conn_test.go

@@ -171,6 +171,8 @@ func TestTimeoutWithoutRead(t *testing.T) {
 }
 
 func testTimeout(t *testing.T, withRead bool) {
+	t.Helper()
+
 	addr, err := net.ResolveUDPAddr("udp", ":0")
 	require.NoError(t, err)
 
@@ -312,6 +314,8 @@ func TestShutdown(t *testing.T) {
 // It fatals if the read blocks longer than timeout,
 // which is useful to detect regressions that would make a test wait forever.
 func requireEcho(t *testing.T, data string, conn io.ReadWriter, timeout time.Duration) {
+	t.Helper()
+
 	_, err := conn.Write([]byte(data))
 	require.NoError(t, err)
 

pkg/udp/proxy_test.go

@@ -41,6 +41,8 @@ func TestUDPProxy(t *testing.T) {
 }
 
 func newServer(t *testing.T, addr string, handler Handler) {
+	t.Helper()
+
 	addrL, err := net.ResolveUDPAddr("udp", addr)
 	require.NoError(t, err)