fix: duplicate port translation for merged gateways

Signed-off-by: Karol Szwaj <[email protected]>
envoyproxy · Mar 29, 2024 · 1caf9f3 · 1caf9f3
1 parent 519eb70
commit 1caf9f3
Show file tree

Hide file tree

Showing 4 changed files with 100 additions and 53 deletions.
diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go
@@ -24,6 +24,8 @@ type ListenersTranslator interface {
 }
 
 func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap, infraIR InfraIRMap, resources *Resources) {
+	// Infra IR proxy ports must be unique across merged gateways.
+	var mergedGatewayPorts []*protocolPort
 	t.validateConflictedLayer7Listeners(gateways)
 	t.validateConflictedLayer4Listeners(gateways, gwapiv1.TCPProtocolType, gwapiv1.TLSProtocolType)
 	t.validateConflictedLayer4Listeners(gateways, gwapiv1.UDPProtocolType)
@@ -36,7 +38,7 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap
 	// to the Xds IR.
 	for _, gateway := range gateways {
 		// Infra IR proxy ports must be unique.
-		var foundPorts []*protocolPort
+		var gatewayPorts []*protocolPort
 		irKey := t.getIRKey(gateway.Gateway)
 
 		if resources.EnvoyProxy != nil {
@@ -93,7 +95,6 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap
 			if !isReady {
 				continue
 			}
-
 			// Add the listener to the Xds IR
 			servicePort := &protocolPort{protocol: listener.Protocol, port: int32(listener.Port)}
 			containerPort := servicePortToContainerPort(servicePort.port)
@@ -120,44 +121,57 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR XdsIRMap
 				xdsIR[irKey].HTTP = append(xdsIR[irKey].HTTP, irListener)
 			}
 
-			// Add the listener to the Infra IR. Infra IR ports must have a unique port number per layer-4 protocol
-			// (TCP or UDP).
-			if !containsPort(foundPorts, servicePort) {
-				foundPorts = append(foundPorts, servicePort)
-				var proto ir.ProtocolType
-				switch listener.Protocol {
-				case gwapiv1.HTTPProtocolType:
-					proto = ir.HTTPProtocolType
-				case gwapiv1.HTTPSProtocolType:
-					proto = ir.HTTPSProtocolType
-				case gwapiv1.TLSProtocolType:
-					proto = ir.TLSProtocolType
-				case gwapiv1.TCPProtocolType:
-					proto = ir.TCPProtocolType
-				case gwapiv1.UDPProtocolType:
-					proto = ir.UDPProtocolType
-				}
+			conflictedPorts := t.processPorts(servicePort, gatewayPorts, mergedGatewayPorts)
+			if !conflictedPorts {
+				t.processInfraIRListener(listener, infraIR, irKey, servicePort)
+				gatewayPorts = append(gatewayPorts, servicePort)
+				mergedGatewayPorts = append(mergedGatewayPorts, servicePort)
+			}
+		}
+	}
+}
+func (t *Translator) processInfraIRListener(listener *ListenerContext, infraIR InfraIRMap, irKey string, servicePort *protocolPort) {
+	// Add the listener to the Infra IR. Infra IR ports must have a unique port number per layer-4 protocol
+	// (TCP or UDP).
+	var proto ir.ProtocolType
+	switch listener.Protocol {
+	case gwapiv1.HTTPProtocolType:
+		proto = ir.HTTPProtocolType
+	case gwapiv1.HTTPSProtocolType:
+		proto = ir.HTTPSProtocolType
+	case gwapiv1.TLSProtocolType:
+		proto = ir.TLSProtocolType
+	case gwapiv1.TCPProtocolType:
+		proto = ir.TCPProtocolType
+	case gwapiv1.UDPProtocolType:
+		proto = ir.UDPProtocolType
+	}
 
-				infraPortName := string(listener.Name)
-				if t.MergeGateways {
-					infraPortName = irHTTPListenerName(listener)
-				}
-				infraPort := ir.ListenerPort{
-					Name:          infraPortName,
-					Protocol:      proto,
-					ServicePort:   servicePort.port,
-					ContainerPort: containerPort,
-				}
+	infraPortName := string(listener.Name)
+	if t.MergeGateways {
+		infraPortName = irHTTPListenerName(listener)
+	}
+	infraPort := ir.ListenerPort{
+		Name:          infraPortName,
+		Protocol:      proto,
+		ServicePort:   servicePort.port,
+		ContainerPort: servicePortToContainerPort(servicePort.port),
+	}
 
-				proxyListener := &ir.ProxyListener{
-					Name:  irHTTPListenerName(listener),
-					Ports: []ir.ListenerPort{infraPort},
-				}
+	proxyListener := &ir.ProxyListener{
+		Name:  irHTTPListenerName(listener),
+		Ports: []ir.ListenerPort{infraPort},
+	}
 
-				infraIR[irKey].Proxy.Listeners = append(infraIR[irKey].Proxy.Listeners, proxyListener)
-			}
-		}
+	infraIR[irKey].Proxy.Listeners = append(infraIR[irKey].Proxy.Listeners, proxyListener)
+}
+
+func (t *Translator) processPorts(servicePort *protocolPort, gatewayPorts, mergedGatewayPorts []*protocolPort) bool {
+	conflictedPorts := containsPort(gatewayPorts, servicePort)
+	if t.MergeGateways {
+		conflictedPorts = containsPort(mergedGatewayPorts, servicePort)
 	}
+	return conflictedPorts
 }
 
 func processAccessLog(envoyproxy *egv1a1.EnvoyProxy) *ir.AccessLog {

diff --git a/internal/gatewayapi/testdata/conflicting-policies.out.yaml b/internal/gatewayapi/testdata/conflicting-policies.out.yaml
@@ -218,13 +218,6 @@ infraIR:
           name: default/gateway-1/http
           protocol: HTTP
           servicePort: 80
-      - address: null
-        name: default/mfqjpuycbgjrtdww/http
-        ports:
-        - containerPort: 10080
-          name: default/mfqjpuycbgjrtdww/http
-          protocol: HTTP
-          servicePort: 80
       metadata:
         labels:
           gateway.envoyproxy.io/owning-gatewayclass: envoy-gateway-class

diff --git a/internal/gatewayapi/testdata/merge-valid-multiple-gateways.in.yaml b/internal/gatewayapi/testdata/merge-valid-multiple-gateways.in.yaml
@@ -21,6 +21,10 @@ gateways:
           allowedRoutes:
             namespaces:
               from: Same
+        - name: http-2
+          hostname: company.com
+          port: 8888
+          protocol: HTTP
   - apiVersion: gateway.networking.k8s.io/v1beta1
     kind: Gateway
     metadata:
@@ -29,13 +33,13 @@ gateways:
     spec:
       gatewayClassName: envoy-gateway-class
       listeners:
-        - name: http-2
+        - name: http-3
           port: 8888
           protocol: HTTP
           allowedRoutes:
             namespaces:
               from: Same
-        - name: http-3
+        - name: http-4
           hostname: example.com
           port: 8888
           protocol: HTTP
diff --git a/internal/gatewayapi/testdata/merge-valid-multiple-gateways.out.yaml b/internal/gatewayapi/testdata/merge-valid-multiple-gateways.out.yaml
@@ -14,6 +14,10 @@ gateways:
       name: http
       port: 80
       protocol: HTTP
+    - hostname: company.com
+      name: http-2
+      port: 8888
+      protocol: HTTP
   status:
     listeners:
     - attachedRoutes: 0
@@ -39,6 +43,29 @@ gateways:
         kind: HTTPRoute
       - group: gateway.networking.k8s.io
         kind: GRPCRoute
+    - attachedRoutes: 0
+      conditions:
+      - lastTransitionTime: null
+        message: Sending translated listener configuration to the data plane
+        reason: Programmed
+        status: "True"
+        type: Programmed
+      - lastTransitionTime: null
+        message: Listener has been successfully translated
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Listener references have been resolved
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      name: http-2
+      supportedKinds:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+      - group: gateway.networking.k8s.io
+        kind: GRPCRoute
 - apiVersion: gateway.networking.k8s.io/v1beta1
   kind: Gateway
   metadata:
@@ -51,11 +78,11 @@ gateways:
     - allowedRoutes:
         namespaces:
           from: Same
-      name: http-2
+      name: http-3
       port: 8888
       protocol: HTTP
     - hostname: example.com
-      name: http-3
+      name: http-4
       port: 8888
       protocol: HTTP
   status:
@@ -77,7 +104,7 @@ gateways:
         reason: ResolvedRefs
         status: "True"
         type: ResolvedRefs
-      name: http-2
+      name: http-3
       supportedKinds:
       - group: gateway.networking.k8s.io
         kind: HTTPRoute
@@ -100,7 +127,7 @@ gateways:
         reason: ResolvedRefs
         status: "True"
         type: ResolvedRefs
-      name: http-3
+      name: http-4
       supportedKinds:
       - group: gateway.networking.k8s.io
         kind: HTTPRoute
@@ -129,10 +156,10 @@ infraIR:
           protocol: HTTP
           servicePort: 80
       - address: null
-        name: envoy-gateway/gateway-2/http-2
+        name: envoy-gateway/gateway-1/http-2
         ports:
         - containerPort: 8888
-          name: envoy-gateway/gateway-2/http-2
+          name: envoy-gateway/gateway-1/http-2
           protocol: HTTP
           servicePort: 8888
       metadata:
@@ -154,11 +181,20 @@ xdsIR:
         escapedSlashesAction: UnescapeAndRedirect
         mergeSlashes: true
       port: 10080
+    - address: 0.0.0.0
+      hostnames:
+      - company.com
+      isHTTP2: false
+      name: envoy-gateway/gateway-1/http-2
+      path:
+        escapedSlashesAction: UnescapeAndRedirect
+        mergeSlashes: true
+      port: 8888
     - address: 0.0.0.0
       hostnames:
       - '*'
       isHTTP2: false
-      name: envoy-gateway/gateway-2/http-2
+      name: envoy-gateway/gateway-2/http-3
       path:
         escapedSlashesAction: UnescapeAndRedirect
         mergeSlashes: true
@@ -167,7 +203,7 @@ xdsIR:
       hostnames:
       - example.com
       isHTTP2: false
-      name: envoy-gateway/gateway-2/http-3
+      name: envoy-gateway/gateway-2/http-4
       path:
         escapedSlashesAction: UnescapeAndRedirect
         mergeSlashes: true