allow same port to used on both public and private ingresses (#1539)

Author: akshaysngupta

pkg/appgw/configbuilder.go

@@ -254,8 +254,8 @@ func generateListenerID(ingress *networking.Ingress, rule *networking.IngressRul
 	}
 
 	extendedHostNames, err := annotations.GetHostNameExtensions(ingress)
-	if err != nil {
-		klog.V(5).Infof("Error while parsing hostname extension: %s", err)
+	if err != nil && !controllererrors.IsErrorCode(err, controllererrors.ErrorMissingAnnotation) {
+		klog.V(5).Infof("Error while parsing host name extensions: %s", err)
 	} else {
 		if extendedHostNames != nil {
 			hostNames = append(hostNames, extendedHostNames...)

pkg/appgw/frontend_listeners.go

@@ -23,7 +23,6 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App
 		return c.mem.listeners, c.mem.ports
 	}
 
-	publIPPorts := make(map[string]string)
 	portsByNumber := cbCtx.ExistingPortsByNumber
 	var listeners []n.ApplicationGatewayHTTPListener
 
@@ -32,7 +31,7 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App
 	}
 
 	if cbCtx.EnvVariables.EnableIstioIntegration {
-		listeners, portsByNumber, publIPPorts = c.getIstioListenersPorts(cbCtx)
+		listeners, portsByNumber = c.getIstioListenersPorts(cbCtx)
 	}
 
 	for listenerID, config := range c.getListenerConfigs(cbCtx) {
@@ -42,15 +41,6 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App
 			continue
 		}
 
-		if listenerName, exists := publIPPorts[*port.Name]; exists && listenerID.FrontendType == FrontendTypePrivate {
-			klog.Errorf("Can't assign port %s to Private IP Listener %s; already assigned to Public IP Listener %s; Will not create listener %+v", *port.Name, *listener.Name, listenerName, listenerID)
-			continue
-		}
-
-		if listenerID.FrontendType == FrontendTypePublic {
-			publIPPorts[*port.Name] = *listener.Name
-		}
-
 		// newlistener created a new port; Add it to the set
 		if _, exists := portsByNumber[Port(*port.Port)]; !exists {
 			portsByNumber[Port(*port.Port)] = *port
@@ -64,6 +54,7 @@ func (c *appGwConfigBuilder) getListeners(cbCtx *ConfigBuilderContext) (*[]n.App
 				listener.SslProfile = resourceRef(sslProfileID)
 			}
 		}
+
 		if config.FirewallPolicy != "" {
 			listener.FirewallPolicy = &n.SubResource{ID: to.StringPtr(config.FirewallPolicy)}
 		}

pkg/appgw/frontend_listeners_istio.go

@@ -10,8 +10,7 @@ import (
 	"k8s.io/klog/v2"
 )
 
-func (c *appGwConfigBuilder) getIstioListenersPorts(cbCtx *ConfigBuilderContext) ([]n.ApplicationGatewayHTTPListener, map[Port]n.ApplicationGatewayFrontendPort, map[string]string) {
-	publIPPorts := make(map[string]string)
+func (c *appGwConfigBuilder) getIstioListenersPorts(cbCtx *ConfigBuilderContext) ([]n.ApplicationGatewayHTTPListener, map[Port]n.ApplicationGatewayFrontendPort) {
 	portsByNumber := cbCtx.ExistingPortsByNumber
 	var listeners []n.ApplicationGatewayHTTPListener
 
@@ -22,20 +21,12 @@ func (c *appGwConfigBuilder) getIstioListenersPorts(cbCtx *ConfigBuilderContext)
 				klog.Errorf("Failed creating listener %+v: %s", listenerID, err)
 				continue
 			}
-			if listenerName, exists := publIPPorts[*port.Name]; exists && listenerID.FrontendType == FrontendTypePrivate {
-				klog.Errorf("Can't assign port %s to Private IP Listener %s; already assigned to Public IP Listener %s", *port.Name, *listener.Name, listenerName)
-				continue
-			}
-
-			if listenerID.FrontendType == FrontendTypePublic {
-				publIPPorts[*port.Name] = *listener.Name
-			}
 
 			listeners = append(listeners, *listener)
 			if _, exists := portsByNumber[Port(*port.Port)]; !exists {
 				portsByNumber[Port(*port.Port)] = *port
 			}
 		}
 	}
-	return listeners, portsByNumber, publIPPorts
+	return listeners, portsByNumber
 }

scripts/e2e/cmd/runner/helper.go

@@ -696,16 +696,23 @@ func getPublicIPForNetworkingV1Ingress(clientset *clientset.Clientset, namespace
 			return "", fmt.Errorf("Unable to find ingress in namespace %s", namespaceName)
 		}
 
-		ingress := (*ingresses).Items[0]
-		if len(ingress.Status.LoadBalancer.Ingress) == 0 {
-			klog.Warning("Trying again in 5 seconds...", i)
-			time.Sleep(5 * time.Second)
-			continue
-		}
+		for _, ingress := range (*ingresses).Items {
+			if ingress.Annotations["appgw.ingress.kubernetes.io/use-private-ip"] == "true" {
+				continue
+			}
 
-		publicIP := ingress.Status.LoadBalancer.Ingress[0].IP
-		if publicIP != "" {
-			return publicIP, nil
+			if len(ingress.Status.LoadBalancer.Ingress) == 0 {
+				klog.Warning("Trying again in 5 seconds...", i)
+				time.Sleep(5 * time.Second)
+				continue
+			}
+
+			publicIP := ingress.Status.LoadBalancer.Ingress[0].IP
+			if publicIP != "" {
+				return publicIP, nil
+			}
+
+			break
 		}
 
 		klog.Warning("getPublicIP: trying again in 5 seconds...", i)

scripts/e2e/cmd/runner/networking-v1-mfu_one_namespace_many_ingresses_test.go

@@ -10,6 +10,7 @@ package runner
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"strconv"
 	"time"
@@ -18,6 +19,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	versioned "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/agic_crd_client/clientset/versioned"
+	n "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2021-03-01/network"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
@@ -221,10 +223,64 @@ var _ = Describe("networking-v1-MFU", func() {
 			Expect(err).To(BeNil())
 		})
 
+		It("[same-port-public-private] ingresses with same port on both public and private IP should work", func() {
+			// create namespace
+			namespaceName = "e2e-same-port-public-private"
+			ns := &v1.Namespace{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: namespaceName,
+				},
+			}
+			klog.Info("Creating namespace: ", namespaceName)
+			_, err = clientset.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{})
+			Expect(err).To(BeNil())
+
+			// create objects in the yaml
+			path := "testdata/networking-v1/one-namespace-many-ingresses/same-port-public-private/app.yaml"
+			klog.Info("Applying yaml: ", path)
+			err = applyYaml(clientset, crdClient, namespaceName, path)
+			Expect(err).To(BeNil())
+
+			var exampleComListeners []n.ApplicationGatewayHTTPListener
+			// Check that gateway has two listeners eventually
+			klog.Info("Checking that gateway has two listeners for hostname example.com...")
+			Eventually(func() bool {
+				appGW, err := getGateway()
+				Expect(err).To(BeNil())
+
+				bytes, _ := json.MarshalIndent(appGW.HTTPListeners, "", "  ")
+				klog.Infof("Listeners: %s", bytes)
+
+				exampleComListeners = []n.ApplicationGatewayHTTPListener{}
+				for _, listener := range *appGW.HTTPListeners {
+					if listener.HostNames == nil {
+						continue
+					}
+
+					if len(*listener.HostNames) == 0 {
+						continue
+					}
+
+					if (*listener.HostNames)[0] == "example.com" {
+						exampleComListeners = append(exampleComListeners, listener)
+					}
+				}
+
+				return len(exampleComListeners) == 2
+			}, 60*time.Second, 5*time.Second).Should(BeTrue())
+
+			// Check that both listeners have the same frontend port
+			klog.Info("Checking that both listeners have the same frontend port...")
+			Expect(exampleComListeners[0].FrontendPort.ID).To(Equal(exampleComListeners[1].FrontendPort.ID))
+
+			// Check that both listeners have the different frontend IP
+			klog.Info("Checking that both listeners have the different frontend IP...")
+			Expect(exampleComListeners[0].FrontendIPConfiguration.ID).ToNot(Equal(exampleComListeners[1].FrontendIPConfiguration.ID))
+		})
+
 		AfterEach(func() {
 			// clear all namespaces
 			cleanUp(clientset)
 		})
 	})
-
 })

scripts/e2e/cmd/runner/testdata/networking-v1/one-namespace-many-ingresses/same-port-public-private/app.yaml

@@ -0,0 +1,87 @@
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: ing-public
+spec:
+  ingressClassName: azure-application-gateway
+  rules:
+  - host: example.com
+    http:
+      paths:
+      - backend:
+          service:
+            name: svc
+            port:
+              name: pp
+        path: /public
+        pathType: Prefix
+---
+apiVersion: networking.k8s.io/v1
+kind: Ingress
+metadata:
+  name: ing1-private
+  annotations:
+    appgw.ingress.kubernetes.io/use-private-ip: "true"
+spec:
+  ingressClassName: azure-application-gateway
+  rules:
+  - host: example.com
+    http:
+      paths:
+      - backend:
+          service:
+            name: svc
+            port:
+              name: pp
+        path: /private
+        pathType: Prefix
+---
+apiVersion: v1
+kind: Service
+metadata:
+  name: svc
+spec:
+  selector:
+    app: app
+  ports:
+  - name: pp
+    protocol: TCP
+    port: 80
+    targetPort: port
+---
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: depl
+spec:
+  selector:
+    matchLabels:
+      app: app
+  replicas: 1
+  template:
+    metadata:
+      labels:
+        app: app
+    spec:
+      containers:
+        - name: ctr
+          imagePullPolicy: Always
+          image: docker.io/kennethreitz/httpbin
+          ports:
+            - name: port
+              containerPort: 80
+          livenessProbe:
+            httpGet:
+              path: /status/200
+              port: 80
+            initialDelaySeconds: 3
+            periodSeconds: 3
+          resources:
+            requests:
+              cpu: 250m
+              memory: 64Mi
+            limits:
+              cpu: 500m
+              memory: 128Mi
+      imagePullSecrets:
+        - name: acr-creds