BUG/MEDIUM: servicePort should be resolved before constructing

backendName

We used to construct backendName with either:
- service.Namespace-service.Name-path.ServicePortString
- service.Namespace-service.Name-path.ServicePortInt
Depending on the format of servicePort provided in Ingress Rule.
The problem with this implementation is that we can have two backends
pointing to the same endpoints (one with PortString and one with PortInt)
while only one backendName can be associated to PortEndpoints for
runtime updates. This leads to potential backend desync at runtime.
This has been fixed by resolving ServicePort from Ingress Rule to get
the corresponding servicePort name and use it in backendName:
- service.Namespace-service.Name-service.Port.Name

Author: Mo3m3n

controller/controller.go

@@ -209,11 +209,11 @@ func (c *HAProxyController) updateHAProxy() {
 			}
 			// Default Backend
 			if ingress.DefaultBackend != nil {
-				c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
+				logger.Error(c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
 					Namespace: namespace,
 					Ingress:   ingress,
 					Path:      ingress.DefaultBackend,
-				})
+				}))
 			}
 			// Ingress secrets
 			logger.Tracef("ingress '%s/%s': processing secrets...", ingress.Namespace, ingress.Name)
@@ -241,14 +241,14 @@ func (c *HAProxyController) updateHAProxy() {
 			logger.Tracef("ingress '%s/%s': processing rules...", ingress.Namespace, ingress.Name)
 			for _, rule := range ingress.Rules {
 				for _, path := range rule.Paths {
-					c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
+					logger.Error(c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
 						Namespace:      namespace,
 						Ingress:        ingress,
 						Host:           rule.Host,
 						Path:           path,
 						HAProxyRules:   c.cfg.HAProxyRules.GetIngressRuleIDs(ingress.Name),
 						SSLPassthrough: c.sslPassthroughEnabled(namespace, ingress, path),
-					})
+					}))
 				}
 			}
 		}
@@ -451,13 +451,14 @@ func (c *HAProxyController) handlePprof() (err error) {
 		return err
 	}
 	logger.Debug("pprof backend created")
-	c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
+	logger.Error(c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
 		Path: &store.IngressPath{
 			Path:           "/debug/pprof",
 			ExactPathMatch: false,
 		},
-		BackendName: pprofBackend,
-	})
+		BackendName:  pprofBackend,
+		LocalBackend: true,
+	}))
 	return nil
 }
 
@@ -497,11 +498,11 @@ func (c *HAProxyController) handleDefaultService() {
 		ServicePortInt:   service.Ports[0].Port,
 		IsDefaultBackend: true,
 	}
-	c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
+	logger.Error(c.cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
 		Namespace: namespace,
 		Ingress:   ingress,
 		Path:      path,
-	})
+	}))
 }
 
 // handleDefaultCert configures default/fallback HAProxy certificate to use for client HTTPS requests.

controller/handler-tcp.go

@@ -116,12 +116,12 @@ func (t TCPHandler) Update(k store.K8s, cfg *Configuration, api api.HAProxyClien
 			Status:         svc.Status,
 		}
 		nsmmp := k.GetNamespace(namespace)
-		cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
+		logger.Error(cfg.IngressRoutes.AddRoute(&ingressRoute.Route{
 			Namespace:  nsmmp,
 			Ingress:    ingress,
 			Path:       path,
 			TCPService: true,
-		})
+		}))
 	}
 	return reload, err
 }

controller/ingress/endpoints.go

@@ -188,19 +188,12 @@ func (route *Route) getEndpoints() {
 		}
 		return
 	}
-	for _, sp := range route.service.Ports {
-		if sp.Name == route.Path.ServicePortString || sp.Port == route.Path.ServicePortInt {
-			if endpoints, ok := endpoints.Ports[sp.Name]; ok {
-				route.endpoints = endpoints
-				return
-			}
-			logger.Warningf("ingress %s/%s: no matching endpoints for service '%s' and port '%s'", route.Namespace.Name, route.Ingress.Name, route.service.Name, sp.Name)
+	if route.Path.ResolvedSvcPort != "" {
+		portName := route.Path.ResolvedSvcPort
+		if endpoints, ok := endpoints.Ports[portName]; ok {
+			route.endpoints = endpoints
 			return
 		}
 	}
-	ingressPort := route.Path.ServicePortString
-	if route.Path.ServicePortInt != 0 {
-		ingressPort = fmt.Sprintf("%d", route.Path.ServicePortInt)
-	}
-	logger.Warningf("ingress %s/%s: service %s: no service port matching '%s'", route.Namespace.Name, route.Ingress.Name, route.service.Name, ingressPort)
+	logger.Warningf("ingress %s/%s: no matching endpoints for service '%s' and port '%d:%s'", route.Namespace.Name, route.Ingress.Name, route.service.Name, route.Path.ServicePortInt, route.Path.ServicePortString)
 }

controller/ingress/route.go

@@ -37,6 +37,7 @@ type Route struct {
 	Host           string
 	BackendName    string
 	NewBackend     bool
+	LocalBackend   bool
 	SSLPassthrough bool
 	TCPService     bool
 	reload         bool
@@ -81,15 +82,8 @@ func (route *Route) addToMapFile(mapFiles haproxy.Maps) error {
 	return nil
 }
 
-// handleService processes an Ingress Route and make corresponding backend configuration in HAProxy
-func (route *Route) handleService() (err error) {
-	// Set backendName
-	if route.Path.ServicePortInt == 0 {
-		route.BackendName = fmt.Sprintf("%s-%s-%s", route.Namespace.Name, route.service.Name, route.Path.ServicePortString)
-	} else {
-		route.BackendName = fmt.Sprintf("%s-%s-%d", route.Namespace.Name, route.service.Name, route.Path.ServicePortInt)
-	}
-
+// handleBackend processes an Ingress Route and makes corresponding backend configuration in HAProxy
+func (route *Route) handleBackend() (err error) {
 	// Get/Create Backend
 	var backend models.Backend
 	if backend, err = client.BackendGet(route.BackendName); err != nil {
@@ -105,7 +99,6 @@ func (route *Route) handleService() (err error) {
 		route.NewBackend = true
 		route.reload = true
 	}
-
 	// Update Backend
 	var switchMode bool
 	if backend.Mode == "http" {
@@ -137,6 +130,29 @@ func (route *Route) handleService() (err error) {
 	return nil
 }
 
+// SetBackendName checks if Ingress ServiceName and ServicePort exists and construct corresponding backend name
+func (route *Route) SetBackendName() (err error) {
+	route.service = route.Namespace.Services[route.Path.ServiceName]
+	if route.service == nil {
+		return fmt.Errorf("ingress %s/%s: service '%s' not found", route.Namespace.Name, route.Ingress.Name, route.Path.ServiceName)
+	}
+	route.Path.ResolvedSvcPort = ""
+	for _, sp := range route.service.Ports {
+		if sp.Name == route.Path.ServicePortString || sp.Port == route.Path.ServicePortInt {
+			route.Path.ResolvedSvcPort = sp.Name
+			break
+		}
+	}
+	if route.Path.ResolvedSvcPort == "" {
+		if route.Path.ServicePortInt != 0 {
+			return fmt.Errorf("ingress %s/%s: service %s: no service port matching '%d'", route.Namespace.Name, route.Ingress.Name, route.service.Name, route.Path.ServicePortInt)
+		}
+		return fmt.Errorf("ingress %s/%s: service %s: no service port matching '%s'", route.Namespace.Name, route.Ingress.Name, route.service.Name, route.Path.ServicePortString)
+	}
+	route.BackendName = fmt.Sprintf("%s-%s-%s", route.service.Namespace, route.service.Name, route.Path.ResolvedSvcPort)
+	return nil
+}
+
 func (route *Route) setStatus() {
 	if route.Path.Status == DELETED || route.service.Status == DELETED {
 		route.status = DELETED

controller/ingress/routes.go

@@ -54,11 +54,12 @@ const (
 	PATH_PREFIX = "path-prefix"
 )
 
-func (r *Routes) AddRoute(route *Route) {
-	route.service = route.Namespace.Services[route.Path.ServiceName]
-	if route.service == nil {
-		logger.Warningf("ingress %s/%s: service '%s' not found", route.Namespace.Name, route.Ingress.Name, route.Path.ServiceName)
-		return
+func (r *Routes) AddRoute(route *Route) (err error) {
+	if route.BackendName == "" {
+		err = route.SetBackendName()
+		if err != nil {
+			return err
+		}
 	}
 	route.setStatus()
 	switch {
@@ -69,6 +70,7 @@ func (r *Routes) AddRoute(route *Route) {
 	default:
 		r.http = append(r.http, route)
 	}
+	return nil
 }
 
 func (r *Routes) Refresh(c api.HAProxyClient, k store.K8s, mapFiles haproxy.Maps, certs *haproxy.Certificates) (reload bool, activeBackends map[string]struct{}) {
@@ -90,11 +92,8 @@ func (r *Routes) refreshHTTP(mapFiles haproxy.Maps) {
 			r.reload = true
 			continue
 		}
-		// Configure Route backend
-		// BackendName != "" then it is a local backend
-		// Example: Pprof
-		if route.BackendName == "" {
-			if err := route.handleService(); err != nil {
+		if !route.LocalBackend {
+			if err := route.handleBackend(); err != nil {
 				logger.Error(err)
 				continue
 			}
@@ -115,7 +114,7 @@ func (r *Routes) refreshHTTPDefault() {
 	// pick latest pushed default route
 	for _, route := range r.httpDefault {
 		if route.status != DELETED {
-			err := route.handleService()
+			err := route.handleBackend()
 			if err != nil {
 				logger.Error(err)
 				continue
@@ -154,7 +153,7 @@ func (r *Routes) refreshTCP() {
 		if route.status == DELETED {
 			continue
 		}
-		if err := route.handleService(); err != nil {
+		if err := route.handleBackend(); err != nil {
 			logger.Error(err)
 			continue
 		}

controller/store/types.go

@@ -32,7 +32,7 @@ type HAProxySrv struct {
 // PortEndpoints describes endpionts of a service port
 type PortEndpoints struct {
 	Port            int64
-	BackendName     string
+	BackendName     string // For runtime operations
 	DynUpdateFailed bool
 	AddrCount       int
 	AddrNew         map[string]struct{}
@@ -83,6 +83,7 @@ type IngressPath struct {
 	ServiceName       string
 	ServicePortInt    int64
 	ServicePortString string
+	ResolvedSvcPort   string
 	Path              string
 	ExactPathMatch    bool
 	IsDefaultBackend  bool