Added support for ingress-level listener port annotations

piyush-tiwari · piyush-tiwari · commit e45f992f630f · 2024-06-06T13:29:49.000+05:30
- Added Ingress level listener port annotations, allowing users to specify a single port to multiplex their http(s) traffic through for an ingress
- Fixed default backend determination
diff --git a/pkg/controllers/backend/backend.go b/pkg/controllers/backend/backend.go
@@ -217,7 +217,7 @@ func (c *Controller) getDefaultBackends(ingresses []*networkingv1.Ingress) ([]oc
 
 	for _, ingress := range ingresses {
 		if ingress.Spec.DefaultBackend != nil {
-			if backend != nil && backend != ingress.Spec.DefaultBackend {
+			if backend != nil && !util.IsBackendServiceEqual(backend, ingress.Spec.DefaultBackend) {
 				return nil, fmt.Errorf("conflict in default backend resource, only one is permitted")
 			}
 			backend = ingress.Spec.DefaultBackend
diff --git a/pkg/controllers/nodeBackend/nodeBackend.go b/pkg/controllers/nodeBackend/nodeBackend.go
@@ -283,7 +283,7 @@ func (c *Controller) getDefaultBackends(ingresses []*networkingv1.Ingress) ([]oc
 
 	for _, ingress := range ingresses {
 		if ingress.Spec.DefaultBackend != nil {
-			if backend != nil && backend != ingress.Spec.DefaultBackend {
+			if backend != nil && !util.IsBackendServiceEqual(backend, ingress.Spec.DefaultBackend) {
 				return nil, fmt.Errorf("conflict in default backend resource, only one is permitted")
 			}
 			backend = ingress.Spec.DefaultBackend
diff --git a/pkg/controllers/routingpolicy/util.go b/pkg/controllers/routingpolicy/util.go
@@ -11,6 +11,7 @@ package routingpolicy
 
 import (
 	"fmt"
+	"github.com/pkg/errors"
 	"strings"
 
 	"github.com/oracle/oci-native-ingress-controller/pkg/util"
@@ -74,17 +75,33 @@ func PathToRoutePolicyCondition(host string, path networkingv1.HTTPIngressPath)
 
 func processRoutingPolicy(ingresses []*networkingv1.Ingress, serviceLister corelisters.ServiceLister, listenerPaths map[string][]*listenerPath, desiredRoutingPolicies sets.String) error {
 	for _, ingress := range ingresses {
+		tlsConfiguredHosts := sets.NewString()
+		for tlsIdx := range ingress.Spec.TLS {
+			ingressTls := ingress.Spec.TLS[tlsIdx]
+			for hostIdx := range ingressTls.Hosts {
+				tlsConfiguredHosts.Insert(ingressTls.Hosts[hostIdx])
+			}
+		}
+
 		for _, rule := range ingress.Spec.Rules {
+			host := rule.Host
+
 			for _, path := range rule.HTTP.Paths {
 				serviceName, servicePort, err := util.PathToServiceAndPort(ingress.Namespace, path, serviceLister)
 				if err != nil {
 					return err
 				}
+
+				listenerPort, err := util.DetermineListenerPort(ingress, &tlsConfiguredHosts, host, servicePort)
+				if err != nil {
+					return errors.Wrap(err, "error determining listener port")
+				}
+
+				listenerName := util.GenerateListenerName(listenerPort)
 				rulePath := path
-				listenerName := util.GenerateListenerName(servicePort)
 				listenerPaths[listenerName] = append(listenerPaths[listenerName], &listenerPath{
 					IngressName:    ingress.Name,
-					Host:           rule.Host,
+					Host:           host,
 					Path:           &rulePath,
 					BackendSetName: util.GenerateBackendSetName(ingress.Namespace, serviceName, servicePort),
 				})
diff --git a/pkg/state/ingressstate.go b/pkg/state/ingressstate.go
@@ -111,6 +111,7 @@ func (s *StateStore) BuildState(ingressClass *networkingv1.IngressClass) error {
 
 	for _, ing := range ingressGroup {
 		hostSecretMap := make(map[string]string)
+		tlsConfiguredHosts := sets.NewString()
 		desiredPorts := sets.NewInt32()
 		// we always expect the default_ingress backendset
 		desiredBackendSets := sets.NewString(DefaultIngressName)
@@ -119,24 +120,33 @@ func (s *StateStore) BuildState(ingressClass *networkingv1.IngressClass) error {
 			ingressTls := ing.Spec.TLS[ingressItem]
 			for j := range ingressTls.Hosts {
 				host := ingressTls.Hosts[j]
+				tlsConfiguredHosts.Insert(host)
 				hostSecretMap[host] = ingressTls.SecretName
 			}
 		}
 
 		for _, rule := range ing.Spec.Rules {
+			host := rule.Host
+
 			for _, path := range rule.HTTP.Paths {
 				serviceName, servicePort, err := util.PathToServiceAndPort(ing.Namespace, path, s.ServiceLister)
 				if err != nil {
 					return errors.Wrap(err, "error finding service and port")
 				}
 
-				desiredPorts.Insert(servicePort)
-				allListeners.Insert(servicePort)
+				listenerPort, err := util.DetermineListenerPort(ing, &tlsConfiguredHosts, host, servicePort)
+				if err != nil {
+					return errors.Wrap(err, "error determining listener port")
+				}
+
+				desiredPorts.Insert(listenerPort)
+				allListeners.Insert(listenerPort)
+
 				bsName := util.GenerateBackendSetName(ing.Namespace, serviceName, servicePort)
 				desiredBackendSets.Insert(bsName)
 				allBackendSets.Insert(bsName)
 
-				err = validateListenerProtocol(ing, listenerProtocolMap, servicePort)
+				err = validateListenerProtocol(ing, listenerProtocolMap, listenerPort)
 				if err != nil {
 					return err
 				}
@@ -153,9 +163,9 @@ func (s *StateStore) BuildState(ingressClass *networkingv1.IngressClass) error {
 				bsTLSEnabled := util.GetBackendTlsEnabled(ing)
 				certificateId := util.GetListenerTlsCertificateOcid(ing)
 				if certificateId != nil {
-					tlsPortDetail, ok := listenerTLSConfigMap[servicePort]
+					tlsPortDetail, ok := listenerTLSConfigMap[listenerPort]
 					if ok {
-						err = validatePortInUse(tlsPortDetail, "", certificateId, servicePort)
+						err = validatePortInUse(tlsPortDetail, "", certificateId, listenerPort)
 						if err != nil {
 							return errors.Wrap(err, "validating certificates")
 						}
@@ -164,17 +174,17 @@ func (s *StateStore) BuildState(ingressClass *networkingv1.IngressClass) error {
 						Type:     ArtifactTypeCertificate,
 						Artifact: *certificateId,
 					}
-					listenerTLSConfigMap[servicePort] = config
+					listenerTLSConfigMap[listenerPort] = config
 					updateBackendTlsStatus(bsTLSEnabled, bsTLSConfigMap, bsName, config)
 				}
 
-				if rule.Host != "" {
-					secretName, ok := hostSecretMap[rule.Host]
+				if host != "" {
+					secretName, ok := hostSecretMap[host]
 
 					if ok && secretName != "" {
-						tlsPortDetail, ok := listenerTLSConfigMap[servicePort]
+						tlsPortDetail, ok := listenerTLSConfigMap[listenerPort]
 						if ok {
-							err = validatePortInUse(tlsPortDetail, secretName, nil, servicePort)
+							err = validatePortInUse(tlsPortDetail, secretName, nil, listenerPort)
 							if err != nil {
 								return errors.Wrap(err, "validating secrets")
 							}
@@ -183,7 +193,7 @@ func (s *StateStore) BuildState(ingressClass *networkingv1.IngressClass) error {
 							Type:     ArtifactTypeSecret,
 							Artifact: secretName,
 						}
-						listenerTLSConfigMap[servicePort] = config
+						listenerTLSConfigMap[listenerPort] = config
 						updateBackendTlsStatus(bsTLSEnabled, bsTLSConfigMap, bsName, config)
 					}
 				}
diff --git a/pkg/util/util.go b/pkg/util/util.go
@@ -15,6 +15,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"fmt"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"strconv"
 	"strings"
 	"time"
@@ -69,13 +70,16 @@ const (
 	IngressHealthCheckReturnCodeAnnotation           = "oci-native-ingress.oraclecloud.com/healthcheck-return-code"
 	IngressHealthCheckResponseBodyRegexAnnotation    = "oci-native-ingress.oraclecloud.com/healthcheck-response-regex"
 	IngressHealthCheckForcePlainTextAnnotation       = "oci-native-ingress.oraclecloud.com/healthcheck-force-plaintext"
+	IngressHttpListenerPortAnnotation                = "oci-native-ingress.oraclecloud.com/http-listener-port"
+	IngressHttpsListenerPortAnnotation               = "oci-native-ingress.oraclecloud.com/https-listener-port"
 
 	ProtocolTCP                            = "TCP"
 	ProtocolHTTP                           = "HTTP"
 	ProtocolHTTP2                          = "HTTP2"
 	ProtocolHTTP2DefaultCipherSuite        = "oci-default-http2-ssl-cipher-suite-v1"
 	DefaultHealthCheckProtocol             = ProtocolTCP
 	DefaultHealthCheckPort                 = 0
+	ZeroPort                               = 0
 	DefaultHealthCheckTimeOutMilliSeconds  = 3000
 	DefaultHealthCheckIntervalMilliSeconds = 10000
 	DefaultHealthCheckRetries              = 3
@@ -270,6 +274,24 @@ func GetIngressHealthCheckForcePlainText(i *networkingv1.Ingress) bool {
 	return result
 }
 
+func GetIngressHttpListenerPort(i *networkingv1.Ingress) (int, error) {
+	value, ok := i.Annotations[IngressHttpListenerPortAnnotation]
+	if !ok {
+		return ZeroPort, nil
+	}
+
+	return strconv.Atoi(value)
+}
+
+func GetIngressHttpsListenerPort(i *networkingv1.Ingress) (int, error) {
+	value, ok := i.Annotations[IngressHttpsListenerPortAnnotation]
+	if !ok {
+		return ZeroPort, nil
+	}
+
+	return strconv.Atoi(value)
+}
+
 func PathToRoutePolicyName(ingressName string, host string, path networkingv1.HTTPIngressPath) string {
 	h := sha256.New()
 	h.Write([]byte(ingressName))
@@ -618,3 +640,38 @@ func RetrievePods(endpointLister corelisters.EndpointsLister, podLister corelist
 	}
 	return pods, nil
 }
+
+func DetermineListenerPort(ingress *networkingv1.Ingress, tlsConfiguredHosts *sets.String, host string, servicePort int32) (int32, error) {
+	annotatedHttpPort, err := GetIngressHttpListenerPort(ingress)
+	if err != nil {
+		return 0, fmt.Errorf("error parsing Ingress Http Listener Port: %w", err)
+	}
+	annotatedHttpsPort, err := GetIngressHttpsListenerPort(ingress)
+	if err != nil {
+		return 0, fmt.Errorf("error parsing Ingress Https Listener Port: %w", err)
+	}
+
+	isCertOcidPresent := GetListenerTlsCertificateOcid(ingress) != nil
+
+	listenerPort := servicePort
+	if isCertOcidPresent || tlsConfiguredHosts.Has(host) {
+		if annotatedHttpsPort != ZeroPort {
+			listenerPort = int32(annotatedHttpsPort)
+		}
+	} else if annotatedHttpPort != ZeroPort {
+		listenerPort = int32(annotatedHttpPort)
+	}
+
+	return listenerPort, nil
+}
+
+// b1 and b2 are assumed non-nil
+func IsBackendServiceEqual(b1 *networkingv1.IngressBackend, b2 *networkingv1.IngressBackend) bool {
+	if b1.Service == b2.Service {
+		return true
+	}
+	if b1.Service == nil || b2.Service == nil || *b1.Service != *b2.Service {
+		return false
+	}
+	return true
+}
diff --git a/pkg/util/util_test.go b/pkg/util/util_test.go
@@ -11,6 +11,7 @@ package util
 import (
 	"context"
 	"fmt"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"strconv"
 	"strings"
 	"testing"
@@ -666,3 +667,80 @@ func TestGetTimeDifferenceInSeconds(t *testing.T) {
 	Expect(GetTimeDifferenceInSeconds(1257894006000, 1257894009000)).Should(Equal(float64(3)))
 	Expect(GetTimeDifferenceInSeconds(1257894006000, 1257894009600)).Should(Equal(3.6))
 }
+
+func TestDetermineListenerPort(t *testing.T) {
+	RegisterTestingT(t)
+	servicePort := int32(8080)
+	httpPort := int32(80)
+	httpsPort := int32(443)
+	tlsConfiguredHosts := sets.NewString("tls-configured-1", "tls-configured-2")
+	ingress := &networkingv1.Ingress{}
+	annotations := map[string]string{}
+	ingress.Annotations = annotations
+
+	listenerPort, err := DetermineListenerPort(ingress, &tlsConfiguredHosts, "not-tls-configured", servicePort)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listenerPort).Should(Equal(servicePort))
+
+	annotations[IngressHttpListenerPortAnnotation] = "80"
+	listenerPort, err = DetermineListenerPort(ingress, &tlsConfiguredHosts, "not-tls-configured", servicePort)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listenerPort).Should(Equal(httpPort))
+
+	listenerPort, err = DetermineListenerPort(ingress, &tlsConfiguredHosts, "tls-configured-1", servicePort)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listenerPort).Should(Equal(servicePort))
+
+	annotations[IngressHttpsListenerPortAnnotation] = "443"
+	listenerPort, err = DetermineListenerPort(ingress, &tlsConfiguredHosts, "tls-configured-1", servicePort)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listenerPort).Should(Equal(httpsPort))
+
+	delete(annotations, IngressHttpsListenerPortAnnotation)
+	annotations[IngressListenerTlsCertificateAnnotation] = "oci_cert"
+	listenerPort, err = DetermineListenerPort(ingress, &tlsConfiguredHosts, "not-tls-configured", servicePort)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listenerPort).Should(Equal(servicePort))
+
+	annotations[IngressHttpsListenerPortAnnotation] = "443"
+	listenerPort, err = DetermineListenerPort(ingress, &tlsConfiguredHosts, "not-tls-configured", servicePort)
+	Expect(err).NotTo(HaveOccurred())
+	Expect(listenerPort).Should(Equal(httpsPort))
+}
+
+func TestIsBackendServiceEqual(t *testing.T) {
+	RegisterTestingT(t)
+	b1 := &networkingv1.IngressBackend{}
+	b2 := &networkingv1.IngressBackend{
+		Service: &networkingv1.IngressServiceBackend{
+			Name: "default-backend-1",
+			Port: networkingv1.ServiceBackendPort{
+				Number: 80,
+			},
+		},
+	}
+	b3 := &networkingv1.IngressBackend{
+		Service: &networkingv1.IngressServiceBackend{
+			Name: "default-backend-1",
+			Port: networkingv1.ServiceBackendPort{
+				Number: 80,
+			},
+		},
+	}
+	b4 := &networkingv1.IngressBackend{
+		Service: &networkingv1.IngressServiceBackend{
+			Name: "default-backend-2",
+			Port: networkingv1.ServiceBackendPort{
+				Number: 80,
+			},
+		},
+	}
+
+	Expect(IsBackendServiceEqual(b2, b3)).To(BeTrue())
+
+	Expect(IsBackendServiceEqual(b1, b2)).To(BeFalse())
+	Expect(IsBackendServiceEqual(b1, b3)).To(BeFalse())
+	Expect(IsBackendServiceEqual(b1, b4)).To(BeFalse())
+	Expect(IsBackendServiceEqual(b2, b4)).To(BeFalse())
+	Expect(IsBackendServiceEqual(b3, b4)).To(BeFalse())
+}

Original file line number	Diff line number	Diff line change
`@@ -217,7 +217,7 @@ func (c Controller) getDefaultBackends(ingresses []networkingv1.Ingress) ([]oc`
`217`	`217`
`218`	`218`	`for _, ingress := range ingresses {`
`219`	`219`	`if ingress.Spec.DefaultBackend != nil {`
`220`		`- if backend != nil && backend != ingress.Spec.DefaultBackend {`
	`220`	`+ if backend != nil && !util.IsBackendServiceEqual(backend, ingress.Spec.DefaultBackend) {`
`221`	`221`	`return nil, fmt.Errorf("conflict in default backend resource, only one is permitted")`
`222`	`222`	`}`
`223`	`223`	`backend = ingress.Spec.DefaultBackend`
Original file line number	Diff line number	Diff line change
`@@ -283,7 +283,7 @@ func (c Controller) getDefaultBackends(ingresses []networkingv1.Ingress) ([]oc`
`283`	`283`
`284`	`284`	`for _, ingress := range ingresses {`
`285`	`285`	`if ingress.Spec.DefaultBackend != nil {`
`286`		`- if backend != nil && backend != ingress.Spec.DefaultBackend {`
	`286`	`+ if backend != nil && !util.IsBackendServiceEqual(backend, ingress.Spec.DefaultBackend) {`
`287`	`287`	`return nil, fmt.Errorf("conflict in default backend resource, only one is permitted")`
`288`	`288`	`}`
`289`	`289`	`backend = ingress.Spec.DefaultBackend`