From 1caf9f3d0b1eddbfad00016ade5b418c4c6e23ea Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Fri, 29 Mar 2024 20:47:29 +0100 Subject: [PATCH] fix: duplicate port translation for merged gateways Signed-off-by: Karol Szwaj --- internal/gatewayapi/listener.go | 86 +++++++++++-------- .../testdata/conflicting-policies.out.yaml | 7 -- .../merge-valid-multiple-gateways.in.yaml | 8 +- .../merge-valid-multiple-gateways.out.yaml | 52 +++++++++-- 4 files changed, 100 insertions(+), 53 deletions(-) diff --git a/internal/gatewayapi/listener.go b/internal/gatewayapi/listener.go index 4702d2a4e26f..b05d8fd18eba 100644 --- 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 index f8afcaee6da6..d8b882d368a0 100644 --- 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 index aad24f222ea3..81e8ae3c9769 100644 --- 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 index 1346f4081150..94ab094ec2ea 100644 --- 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