Merge pull request #13 from serverscom/fix-location-path

fix location path

Author: olegy89

internal/ingress/controller/store/store.go

@@ -37,7 +37,7 @@ type Storer interface {
 	ListIngress() []*networkv1.Ingress
 	GetService(key string) (*corev1.Service, error)
 	GetNodesIpList() []string
-	GetIngressServiceInfo(ingress *networkv1.Ingress) (map[string]ServiceInfo, error)
+	GetIngressHostsInfo(ingress *networkv1.Ingress) (map[string]HostInfo, error)
 }
 
 // Store represents cache store, implements Storer
@@ -75,8 +75,8 @@ func (s *Store) GetNodesIpList() []string {
 }
 
 // GetIngressServiceInfo returns ingress services info.
-func (s *Store) GetIngressServiceInfo(ingress *networkv1.Ingress) (map[string]ServiceInfo, error) {
-	return getIngressServiceInfo(ingress, s)
+func (s *Store) GetIngressHostsInfo(ingress *networkv1.Ingress) (map[string]HostInfo, error) {
+	return getIngressHostsInfo(ingress, s)
 }
 
 type Informer struct {

internal/ingress/controller/store/store_test.go

@@ -1,7 +1,7 @@
 package store
 
 import (
-	"errors"
+	"fmt"
 	"testing"
 	"time"
 
@@ -144,6 +144,7 @@ func TestGetIngressServiceInfo(t *testing.T) {
 	}
 	s.listers.Node.Add(node1)
 
+	hostname := "example.com"
 	serviceName := "test-service"
 	service := &corev1.Service{
 		ObjectMeta: metav1.ObjectMeta{
@@ -172,7 +173,7 @@ func TestGetIngressServiceInfo(t *testing.T) {
 		Spec: networkv1.IngressSpec{
 			Rules: []networkv1.IngressRule{
 				{
-					Host: "example.com",
+					Host: hostname,
 					IngressRuleValue: networkv1.IngressRuleValue{
 						HTTP: &networkv1.HTTPIngressRuleValue{
 							Paths: []networkv1.HTTPIngressPath{
@@ -193,20 +194,22 @@ func TestGetIngressServiceInfo(t *testing.T) {
 		},
 	}
 
-	serviceInfo, err := s.GetIngressServiceInfo(ingress)
+	hostsInfo, err := s.GetIngressHostsInfo(ingress)
 	g.Expect(err).To(BeNil())
+	g.Expect(hostsInfo[hostname].Paths).ToNot(BeEmpty())
 
-	g.Expect(serviceInfo).To(HaveKey(serviceName))
-	g.Expect(serviceInfo[serviceName].Hosts).To(ContainElement("example.com"))
-	g.Expect(serviceInfo[serviceName].NodePort).To(Equal(30000))
-	g.Expect(serviceInfo[serviceName].NodeIps).To(ConsistOf("192.168.1.1"))
-	g.Expect(serviceInfo[serviceName].Annotations).To(HaveKeyWithValue("key", "value"))
+	p := hostsInfo[hostname].Paths[0]
+
+	g.Expect(p.Service.Name).To(Equal(serviceName))
+	g.Expect(p.NodePort).To(Equal(30000))
+	g.Expect(p.NodeIps).To(ConsistOf("192.168.1.1"))
+	g.Expect(p.Service.Annotations).To(HaveKeyWithValue("key", "value"))
 
 	// check if service doens't have NodePort
 	service.Spec.Ports[0].NodePort = 0
 	s.listers.Service.Update(service)
 
-	serviceInfo, err = s.GetIngressServiceInfo(ingress)
-	expectedErr := errors.New("service doesn't have NodePort, only services with type 'NodePort' or 'LoadBalancer' supported")
+	_, err = s.GetIngressHostsInfo(ingress)
+	expectedErr := fmt.Errorf("service %s has no NodePort (only NodePort/LoadBalancer supported)", serviceName)
 	g.Expect(err).To(Equal(expectedErr))
 }

internal/ingress/controller/store/utils.go

@@ -3,57 +3,69 @@ package store
 import (
 	"fmt"
 
+	corev1 "k8s.io/api/core/v1"
 	networkv1 "k8s.io/api/networking/v1"
 )
 
-// ServiceInfo represents helper struct for ingress service
-type ServiceInfo struct {
-	Hosts       []string
-	NodeIps     []string
-	NodePort    int
-	Annotations map[string]string
+// PathInfo represents info about a path in the ingress controller
+type PathInfo struct {
+	Path     string
+	Service  *corev1.Service
+	NodePort int
+	NodeIps  []string
 }
 
-// GetIngressServiceInfo get services info from ingress
-func getIngressServiceInfo(ingress *networkv1.Ingress, store Storer) (map[string]ServiceInfo, error) {
-	servicesInfo := make(map[string]ServiceInfo)
+// HostInfo represents info about a host in the ingress controller
+type HostInfo struct {
+	Host  string
+	Paths []PathInfo
+}
+
+// getIngressHostsInfo get hosts info from ingress
+func getIngressHostsInfo(ingress *networkv1.Ingress, store Storer) (map[string]HostInfo, error) {
+	hostsInfo := make(map[string]HostInfo)
 	nodeIps := store.GetNodesIpList()
 
 	for _, rule := range ingress.Spec.Rules {
 		if rule.HTTP == nil {
 			continue
 		}
 
+		hInfo := hostsInfo[rule.Host]
+		hInfo.Host = rule.Host
+
 		for _, path := range rule.HTTP.Paths {
-			service, err := store.GetService(ingress.Namespace + "/" + path.Backend.Service.Name)
+			svc, err := store.GetService(ingress.Namespace + "/" + path.Backend.Service.Name)
 			if err != nil {
 				return nil, fmt.Errorf("error getting service: %v", err)
 			}
 
-			for _, port := range service.Spec.Ports {
+			var nodePort int32
+			found := false
+			for _, port := range svc.Spec.Ports {
 				if port.Port == path.Backend.Service.Port.Number {
-					if port.NodePort != 0 {
-						serviceName := path.Backend.Service.Name
-						if _, ok := servicesInfo[serviceName]; !ok {
-							servicesInfo[serviceName] = ServiceInfo{
-								Hosts:       []string{rule.Host},
-								NodePort:    int(port.NodePort),
-								NodeIps:     nodeIps,
-								Annotations: service.Annotations,
-							}
-						} else {
-							sTmp := servicesInfo[serviceName]
-							sTmp.Hosts = append(sTmp.Hosts, rule.Host)
-							servicesInfo[serviceName] = sTmp
-						}
-					} else {
-						return nil, fmt.Errorf("service doesn't have NodePort, only services with type 'NodePort' or 'LoadBalancer' supported")
+					if port.NodePort == 0 {
+						return nil, fmt.Errorf("service %s has no NodePort (only NodePort/LoadBalancer supported)", svc.Name)
 					}
+					nodePort = port.NodePort
+					found = true
 					break
 				}
 			}
+			if !found {
+				return nil, fmt.Errorf("service %s: port %d not found", svc.Name, path.Backend.Service.Port.Number)
+			}
+
+			hInfo.Paths = append(hInfo.Paths, PathInfo{
+				Path:     path.Path,
+				Service:  svc,
+				NodePort: int(nodePort),
+				NodeIps:  nodeIps,
+			})
 		}
+
+		hostsInfo[rule.Host] = hInfo
 	}
 
-	return servicesInfo, nil
+	return hostsInfo, nil
 }

internal/mocks/store.go

@@ -145,59 +145,77 @@ func (m *Manager) GetIds() []string {
 func (m *Manager) TranslateIngressToLB(ingress *networkv1.Ingress, sslCerts map[string]string) (*serverscom.L7LoadBalancerCreateInput, error) {
 	m.lock.Lock()
 	defer m.lock.Unlock()
-	sInfo, err := m.store.GetIngressServiceInfo(ingress)
+
+	hostsInfo, err := m.store.GetIngressHostsInfo(ingress)
 	if err != nil {
 		return nil, err
 	}
 
-	var upstreamZones []serverscom.L7UpstreamZoneInput
-	var upstreams []serverscom.L7UpstreamInput
 	var vhostZones []serverscom.L7VHostZoneInput
-	var locationZones []serverscom.L7LocationZoneInput
 
-	for sKey, service := range sInfo {
-		sslId := ""
-		sslEnabled := false
+	upstreamMap := make(map[string]serverscom.L7UpstreamZoneInput)
+
+	for host, hInfo := range hostsInfo {
+		var locationZones []serverscom.L7LocationZoneInput
 		vhostPorts := []int32{80}
-		upstreamId := fmt.Sprintf("upstream-zone-%s", sKey)
-		for _, ip := range service.NodeIps {
-			upstreams = append(upstreams, serverscom.L7UpstreamInput{
-				IP:     ip,
-				Weight: 1,
-				Port:   int32(service.NodePort),
-			})
+		sslEnabled := false
+		sslId := ""
+
+		if id, ok := sslCerts[host]; ok {
+			sslId = id
+			sslEnabled = true
+			vhostPorts = []int32{443}
 		}
 
-		for _, host := range service.Hosts {
-			if id, ok := sslCerts[host]; ok {
-				sslId = id
-				sslEnabled = true
-				vhostPorts = []int32{443}
-			}
+		vhostAnnotations := make(map[string]string)
+		for _, p := range hInfo.Paths {
+			upstreamId := fmt.Sprintf("upstream-zone-%s-%d", p.Service.Name, p.NodePort)
+
 			locationZones = append(locationZones, serverscom.L7LocationZoneInput{
-				Location:   "/",
+				Location:   p.Path,
 				UpstreamID: upstreamId,
 			})
+
+			if _, ok := upstreamMap[upstreamId]; !ok {
+				var ups []serverscom.L7UpstreamInput
+				for _, ip := range p.NodeIps {
+					ups = append(ups, serverscom.L7UpstreamInput{
+						IP:     ip,
+						Port:   int32(p.NodePort),
+						Weight: 1,
+					})
+				}
+				upstream := serverscom.L7UpstreamZoneInput{
+					ID:        upstreamId,
+					Upstreams: ups,
+				}
+				upstream = *annotations.FillLBUpstreamZoneWithServiceAnnotations(&upstream, p.Service.Annotations)
+				upstreamMap[upstreamId] = upstream
+			}
+
+			// last-win strategy for vhost annotations
+			for k, v := range p.Service.Annotations {
+				vhostAnnotations[k] = v
+			}
 		}
 
-		vZInput := serverscom.L7VHostZoneInput{
-			ID:            fmt.Sprintf("vhost-zone-%s", sKey),
-			Domains:       service.Hosts,
+		vz := serverscom.L7VHostZoneInput{
+			ID:            fmt.Sprintf("vhost-zone-%s", host),
+			Domains:       []string{host},
 			SSLCertID:     sslId,
 			SSL:           sslEnabled,
 			Ports:         vhostPorts,
 			LocationZones: locationZones,
 		}
-		vZInput = *annotations.FillLBVHostZoneWithServiceAnnotations(&vZInput, service.Annotations)
-		vhostZones = append(vhostZones, vZInput)
+		vz = *annotations.FillLBVHostZoneWithServiceAnnotations(&vz, vhostAnnotations)
+		vhostZones = append(vhostZones, vz)
+	}
 
-		uZInput := serverscom.L7UpstreamZoneInput{
-			ID:        upstreamId,
-			Upstreams: upstreams,
-		}
-		uZInput = *annotations.FillLBUpstreamZoneWithServiceAnnotations(&uZInput, service.Annotations)
-		upstreamZones = append(upstreamZones, uZInput)
+	var upstreamZones []serverscom.L7UpstreamZoneInput
+	for _, u := range upstreamMap {
+		upstreamZones = append(upstreamZones, u)
 	}
+
 	if len(vhostZones) == 0 || len(upstreamZones) == 0 {
 		return nil, errors.New("vhost or upstream can't be empty, can't continue")
 	}