Improve error on router without service.

Co-authored-by: Mathieu Lonjaret <mathieu.lonjaret@gmail.com>

integration/testdata/rawdata-crd.json

@@ -31,6 +31,7 @@
 					"/tobestripped"
 				]
 			},
+			"status": "enabled",
 			"usedBy": [
 				"default/test2.route-23c7f4c450289ee29016@kubernetescrd"
 			]

pkg/api/handler_overview.go

@@ -100,15 +100,19 @@ func getHTTPServiceSection(services map[string]*runtime.ServiceInfo) *section {
 
 func getHTTPMiddlewareSection(middlewares map[string]*runtime.MiddlewareInfo) *section {
 	var countErrors int
-	for _, md := range middlewares {
+	var countWarnings int
-		if md.Err != nil {
+	for _, mid := range middlewares {
+		switch mid.Status {
+		case runtime.StatusDisabled:
 			countErrors++
+		case runtime.StatusWarning:
+			countWarnings++
 		}
 	}
 
 	return &section{
 		Total:    len(middlewares),
-		Warnings: 0,
+		Warnings: countWarnings,
 		Errors:   countErrors,
 	}
 }

pkg/api/handler_overview_test.go

@@ -85,6 +85,7 @@ func TestHandler_Overview(t *testing.T) {
 								Users: []string{"admin:admin"},
 							},
 						},
+						Status: runtime.StatusEnabled,
 					},
 					"addPrefixTest@myprovider": {
 						Middleware: &dynamic.Middleware{
@@ -99,7 +100,8 @@ func TestHandler_Overview(t *testing.T) {
 								Prefix: "/toto",
 							},
 						},
-						Err: []string{"error"},
+						Err:    []string{"error"},
+						Status: runtime.StatusDisabled,
 					},
 				},
 				Routers: map[string]*runtime.RouterInfo{

pkg/api/testdata/getrawdata.json

@@ -30,6 +30,7 @@
 			"addPrefix": {
 				"prefix": "/toto"
 			},
+			"status": "enabled",
 			"usedBy": [
 				"bar@myprovider"
 			]
@@ -38,6 +39,7 @@
 			"addPrefix": {
 				"prefix": "/titi"
 			},
+			"status": "enabled",
 			"usedBy": [
 				"test@myprovider"
 			]
@@ -48,6 +50,7 @@
 					"admin:admin"
 				]
 			},
+			"status": "enabled",
 			"usedBy": [
 				"bar@myprovider",
 				"test@myprovider"

pkg/api/testdata/middleware-auth.json

@@ -6,6 +6,7 @@
 	},
 	"name": "auth@myprovider",
 	"provider": "myprovider",
+	"status": "enabled",
 	"usedBy": [
 		"bar@myprovider",
 		"test@myprovider"

pkg/api/testdata/middlewares-page2.json

@@ -5,6 +5,7 @@
 		},
 		"name": "addPrefixTest@myprovider",
 		"provider": "myprovider",
+		"status": "enabled",
 		"usedBy": [
 			"test@myprovider"
 		]

pkg/api/testdata/middlewares.json

@@ -5,6 +5,7 @@
 		},
 		"name": "addPrefixTest@anotherprovider",
 		"provider": "anotherprovider",
+		"status": "enabled",
 		"usedBy": [
 			"bar@myprovider"
 		]
@@ -15,6 +16,7 @@
 		},
 		"name": "addPrefixTest@myprovider",
 		"provider": "myprovider",
+		"status": "enabled",
 		"usedBy": [
 			"test@myprovider"
 		]
@@ -27,6 +29,7 @@
 		},
 		"name": "auth@myprovider",
 		"provider": "myprovider",
+		"status": "enabled",
 		"usedBy": [
 			"bar@myprovider",
 			"test@myprovider"

pkg/config/runtime/runtime.go

@@ -55,7 +55,7 @@ func NewConfig(conf dynamic.Configuration) *Configuration {
 		if len(middlewares) > 0 {
 			runtimeConfig.Middlewares = make(map[string]*MiddlewareInfo, len(middlewares))
 			for k, v := range middlewares {
-				runtimeConfig.Middlewares[k] = &MiddlewareInfo{Middleware: v}
+				runtimeConfig.Middlewares[k] = &MiddlewareInfo{Middleware: v, Status: StatusEnabled}
 			}
 		}
 	}
@@ -81,14 +81,14 @@ func NewConfig(conf dynamic.Configuration) *Configuration {
 
 // PopulateUsedBy populates all the UsedBy lists of the underlying fields of r,
 // based on the relations between the included services, routers, and middlewares.
-func (r *Configuration) PopulateUsedBy() {
+func (c *Configuration) PopulateUsedBy() {
-	if r == nil {
+	if c == nil {
 		return
 	}
 
 	logger := log.WithoutContext()
 
-	for routerName, routerInfo := range r.Routers {
+	for routerName, routerInfo := range c.Routers {
 		// lazily initialize Status in case caller forgot to do it
 		if routerInfo.Status == "" {
 			routerInfo.Status = StatusEnabled
@@ -102,33 +102,38 @@ func (r *Configuration) PopulateUsedBy() {
 
 		for _, midName := range routerInfo.Router.Middlewares {
 			fullMidName := getQualifiedName(providerName, midName)
-			if _, ok := r.Middlewares[fullMidName]; !ok {
+			if _, ok := c.Middlewares[fullMidName]; !ok {
 				continue
 			}
-			r.Middlewares[fullMidName].UsedBy = append(r.Middlewares[fullMidName].UsedBy, routerName)
+			c.Middlewares[fullMidName].UsedBy = append(c.Middlewares[fullMidName].UsedBy, routerName)
 		}
 
 		serviceName := getQualifiedName(providerName, routerInfo.Router.Service)
-		if _, ok := r.Services[serviceName]; !ok {
+		if _, ok := c.Services[serviceName]; !ok {
 			continue
 		}
-		r.Services[serviceName].UsedBy = append(r.Services[serviceName].UsedBy, routerName)
+		c.Services[serviceName].UsedBy = append(c.Services[serviceName].UsedBy, routerName)
 	}
 
-	for k, serviceInfo := range r.Services {
+	for k, serviceInfo := range c.Services {
 		// lazily initialize Status in case caller forgot to do it
 		if serviceInfo.Status == "" {
 			serviceInfo.Status = StatusEnabled
 		}
 
-		sort.Strings(r.Services[k].UsedBy)
+		sort.Strings(c.Services[k].UsedBy)
 	}
 
-	for k := range r.Middlewares {
+	for midName, mid := range c.Middlewares {
-		sort.Strings(r.Middlewares[k].UsedBy)
+		// lazily initialize Status in case caller forgot to do it
+		if mid.Status == "" {
+			mid.Status = StatusEnabled
+		}
+
+		sort.Strings(c.Middlewares[midName].UsedBy)
 	}
 
-	for routerName, routerInfo := range r.TCPRouters {
+	for routerName, routerInfo := range c.TCPRouters {
 		// lazily initialize Status in case caller forgot to do it
 		if routerInfo.Status == "" {
 			routerInfo.Status = StatusEnabled
@@ -141,19 +146,19 @@ func (r *Configuration) PopulateUsedBy() {
 		}
 
 		serviceName := getQualifiedName(providerName, routerInfo.TCPRouter.Service)
-		if _, ok := r.TCPServices[serviceName]; !ok {
+		if _, ok := c.TCPServices[serviceName]; !ok {
 			continue
 		}
-		r.TCPServices[serviceName].UsedBy = append(r.TCPServices[serviceName].UsedBy, routerName)
+		c.TCPServices[serviceName].UsedBy = append(c.TCPServices[serviceName].UsedBy, routerName)
 	}
 
-	for k, serviceInfo := range r.TCPServices {
+	for k, serviceInfo := range c.TCPServices {
 		// lazily initialize Status in case caller forgot to do it
 		if serviceInfo.Status == "" {
 			serviceInfo.Status = StatusEnabled
 		}
 
-		sort.Strings(r.TCPServices[k].UsedBy)
+		sort.Strings(c.TCPServices[k].UsedBy)
 	}
 }
 
@@ -167,10 +172,10 @@ func contains(entryPoints []string, entryPointName string) bool {
 }
 
 // GetRoutersByEntryPoints returns all the http routers by entry points name and routers name
-func (r *Configuration) GetRoutersByEntryPoints(ctx context.Context, entryPoints []string, tls bool) map[string]map[string]*RouterInfo {
+func (c *Configuration) GetRoutersByEntryPoints(ctx context.Context, entryPoints []string, tls bool) map[string]map[string]*RouterInfo {
 	entryPointsRouters := make(map[string]map[string]*RouterInfo)
 
-	for rtName, rt := range r.Routers {
+	for rtName, rt := range c.Routers {
 		if (tls && rt.TLS == nil) || (!tls && rt.TLS != nil) {
 			continue
 		}
@@ -198,10 +203,10 @@ func (r *Configuration) GetRoutersByEntryPoints(ctx context.Context, entryPoints
 }
 
 // GetTCPRoutersByEntryPoints returns all the tcp routers by entry points name and routers name
-func (r *Configuration) GetTCPRoutersByEntryPoints(ctx context.Context, entryPoints []string) map[string]map[string]*TCPRouterInfo {
+func (c *Configuration) GetTCPRoutersByEntryPoints(ctx context.Context, entryPoints []string) map[string]map[string]*TCPRouterInfo {
 	entryPointsRouters := make(map[string]map[string]*TCPRouterInfo)
 
-	for rtName, rt := range r.TCPRouters {
+	for rtName, rt := range c.TCPRouters {
 		eps := rt.EntryPoints
 		if len(eps) == 0 {
 			eps = entryPoints
@@ -259,25 +264,47 @@ func (r *RouterInfo) AddError(err error, critical bool) {
 
 // TCPRouterInfo holds information about a currently running TCP router
 type TCPRouterInfo struct {
-	*dynamic.TCPRouter        // dynamic configuration
+	*dynamic.TCPRouter          // dynamic configuration
-	Err                string `json:"error,omitempty"` // initialization error
+	Err                []string `json:"error,omitempty"` // initialization error
 	// Status reports whether the router is disabled, in a warning state, or all good (enabled).
 	// If not in "enabled" state, the reason for it should be in the list of Err.
 	// It is the caller's responsibility to set the initial status.
 	Status string `json:"status,omitempty"`
 }
 
+// AddError adds err to r.Err, if it does not already exist.
+// If critical is set, r is marked as disabled.
+func (r *TCPRouterInfo) AddError(err error, critical bool) {
+	for _, value := range r.Err {
+		if value == err.Error() {
+			return
+		}
+	}
+
+	r.Err = append(r.Err, err.Error())
+	if critical {
+		r.Status = StatusDisabled
+		return
+	}
+
+	// only set it to "warning" if not already in a worse state
+	if r.Status != StatusDisabled {
+		r.Status = StatusWarning
+	}
+}
+
 // MiddlewareInfo holds information about a currently running middleware
 type MiddlewareInfo struct {
 	*dynamic.Middleware // dynamic configuration
 	// Err contains all the errors that occurred during service creation.
 	Err    []string `json:"error,omitempty"`
+	Status string   `json:"status,omitempty"`
 	UsedBy []string `json:"usedBy,omitempty"` // list of routers and services using that middleware
 }
 
 // AddError adds err to s.Err, if it does not already exist.
 // If critical is set, m is marked as disabled.
-func (m *MiddlewareInfo) AddError(err error) {
+func (m *MiddlewareInfo) AddError(err error, critical bool) {
 	for _, value := range m.Err {
 		if value == err.Error() {
 			return
@@ -285,6 +312,15 @@ func (m *MiddlewareInfo) AddError(err error) {
 	}
 
 	m.Err = append(m.Err, err.Error())
+	if critical {
+		m.Status = StatusDisabled
+		return
+	}
+
+	// only set it to "warning" if not already in a worse state
+	if m.Status != StatusDisabled {
+		m.Status = StatusWarning
+	}
 }
 
 // ServiceInfo holds information about a currently running service
@@ -354,8 +390,8 @@ func (s *ServiceInfo) GetAllStatus() map[string]string {
 
 // TCPServiceInfo holds information about a currently running TCP service
 type TCPServiceInfo struct {
-	*dynamic.TCPService       // dynamic configuration
+	*dynamic.TCPService          // dynamic configuration
-	Err                 error `json:"error,omitempty"` // initialization error
+	Err                 []string `json:"error,omitempty"` // initialization error
 	// Status reports whether the service is disabled, in a warning state, or all good (enabled).
 	// If not in "enabled" state, the reason for it should be in the list of Err.
 	// It is the caller's responsibility to set the initial status.
@@ -363,6 +399,27 @@ type TCPServiceInfo struct {
 	UsedBy []string `json:"usedBy,omitempty"` // list of routers using that service
 }
 
+// AddError adds err to s.Err, if it does not already exist.
+// If critical is set, s is marked as disabled.
+func (s *TCPServiceInfo) AddError(err error, critical bool) {
+	for _, value := range s.Err {
+		if value == err.Error() {
+			return
+		}
+	}
+
+	s.Err = append(s.Err, err.Error())
+	if critical {
+		s.Status = StatusDisabled
+		return
+	}
+
+	// only set it to "warning" if not already in a worse state
+	if s.Status != StatusDisabled {
+		s.Status = StatusWarning
+	}
+}
+
 func getProviderName(elementName string) string {
 	parts := strings.Split(elementName, "@")
 	if len(parts) > 1 {

pkg/server/middleware/middlewares.go

@@ -65,19 +65,19 @@ func (b *Builder) BuildChain(ctx context.Context, middlewares []string) *alice.C
 
 			var err error
 			if constructorContext, err = checkRecursion(constructorContext, middlewareName); err != nil {
-				b.configs[middlewareName].AddError(err)
+				b.configs[middlewareName].AddError(err, true)
 				return nil, err
 			}
 
 			constructor, err := b.buildConstructor(constructorContext, middlewareName)
 			if err != nil {
-				b.configs[middlewareName].AddError(err)
+				b.configs[middlewareName].AddError(err, true)
 				return nil, err
 			}
 
 			handler, err := constructor(next)
 			if err != nil {
-				b.configs[middlewareName].AddError(err)
+				b.configs[middlewareName].AddError(err, true)
 				return nil, err
 			}
 

pkg/server/router/router.go

@@ -2,6 +2,7 @@ package router
 
 import (
 	"context"
+	"errors"
 	"net/http"
 
 	"github.com/containous/alice"
@@ -148,6 +149,10 @@ func (m *Manager) buildHTTPHandler(ctx context.Context, router *runtime.RouterIn
 	}
 	rm := m.modifierBuilder.Build(ctx, qualifiedNames)
 
+	if router.Service == "" {
+		return nil, errors.New("the service is missing on the router")
+	}
+
 	sHandler, err := m.serviceManager.BuildHTTP(ctx, router.Service, rm)
 	if err != nil {
 		return nil, err

pkg/server/router/tcp/router.go

@@ -3,6 +3,7 @@ package tcp
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -169,9 +170,16 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string
 		ctxRouter := log.With(internal.AddProviderInContext(ctx, routerName), log.Str(log.RouterName, routerName))
 		logger := log.FromContext(ctxRouter)
 
+		if routerConfig.Service == "" {
+			err := errors.New("the service is missing on the router")
+			routerConfig.AddError(err, true)
+			logger.Error(err)
+			continue
+		}
+
 		handler, err := m.serviceManager.BuildTCP(ctxRouter, routerConfig.Service)
 		if err != nil {
-			routerConfig.Err = err.Error()
+			routerConfig.AddError(err, true)
 			logger.Error(err)
 			continue
 		}
@@ -179,7 +187,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string
 		domains, err := rules.ParseHostSNI(routerConfig.Rule)
 		if err != nil {
 			routerErr := fmt.Errorf("unknown rule %s", routerConfig.Rule)
-			routerConfig.Err = routerErr.Error()
+			routerConfig.AddError(routerErr, true)
 			logger.Debug(routerErr)
 			continue
 		}
@@ -203,7 +211,7 @@ func (m *Manager) buildEntryPointHandler(ctx context.Context, configs map[string
 
 					tlsConf, err := m.tlsManager.Get("default", tlsOptionsName)
 					if err != nil {
-						routerConfig.Err = err.Error()
+						routerConfig.AddError(err, true)
 						logger.Debug(err)
 						continue
 					}

pkg/server/router/tcp/router_test.go

@@ -232,12 +232,11 @@ func TestRuntimeConfiguration(t *testing.T) {
 				}
 			}
 			for _, v := range conf.TCPRouters {
-				if v.Err != "" {
+				if len(v.Err) > 0 {
 					allErrors++
 				}
 			}
 			assert.Equal(t, test.expectedError, allErrors)
 		})
 	}
-
 }

pkg/server/service/tcp/service.go

@@ -35,8 +35,9 @@ func (m *Manager) BuildTCP(rootCtx context.Context, serviceName string) (tcp.Han
 		return nil, fmt.Errorf("the service %q does not exist", serviceQualifiedName)
 	}
 	if conf.LoadBalancer == nil {
-		conf.Err = fmt.Errorf("the service %q doesn't have any TCP load balancer", serviceQualifiedName)
+		err := fmt.Errorf("the service %q doesn't have any TCP load balancer", serviceQualifiedName)
-		return nil, conf.Err
+		conf.AddError(err, true)
+		return nil, err
 	}
 
 	logger := log.FromContext(ctx)