Skip to content

Commit a792820

Browse files
zirainjukie
zirain
and
jukie
committed
skip invalid listener first in IR (envoyproxy#8577)
* skip invalid listener Signed-off-by: zirain <zirain2009@gmail.com> * fix specValid Signed-off-by: zirain <zirain2009@gmail.com> * nit Signed-off-by: zirain <zirain2009@gmail.com> * fix Signed-off-by: zirain <zirain2009@gmail.com> * MUST NOT pick one conflicting Listener as the winner Signed-off-by: zirain <zirain2009@gmail.com> * update Signed-off-by: zirain <zirain2009@gmail.com> --------- Signed-off-by: zirain <zirain2009@gmail.com> Co-authored-by: Isaac Wilson <isaac.wilson514@gmail.com> Signed-off-by: Karol Szwaj <karol.szwaj@gmail.com>
1 parent f8f57b8 commit a792820

8 files changed

Lines changed: 683 additions & 123 deletions

‎internal/gatewayapi/contexts.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,10 @@ type ListenerContext struct {
103103
tlsSecrets []*corev1.Secret
104104
certDNSNames []string
105105

106+
// specValid indicates whether per-listener spec validation succeeded.
107+
// Conflict detection should only consider listeners with specValid=true.
108+
specValid bool
109+
106110
httpIR *ir.HTTPListener
107111
}
108112

‎internal/gatewayapi/listener.go‎

Lines changed: 96 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -38,19 +38,92 @@ type ListenersTranslator interface {
3838
ProcessListeners(gateways []*GatewayContext, xdsIR resource.XdsIRMap, infraIR resource.InfraIRMap, resources *resource.Resources)
3939
}
4040

41+
// allowedRouteKindsForProtocol returns the route kinds supported by the given listener protocol.
42+
func allowedRouteKindsForProtocol(protocol gwapiv1.ProtocolType, tlsMode *gwapiv1.TLSModeType) []gwapiv1.Kind {
43+
switch protocol {
44+
case gwapiv1.HTTPProtocolType, gwapiv1.HTTPSProtocolType:
45+
return []gwapiv1.Kind{resource.KindHTTPRoute, resource.KindGRPCRoute}
46+
case gwapiv1.TLSProtocolType:
47+
if tlsMode != nil && *tlsMode == gwapiv1.TLSModePassthrough {
48+
return []gwapiv1.Kind{resource.KindTLSRoute}
49+
}
50+
// Terminate mode or unspecified defaults to accept both TCP and TLS routes
51+
return []gwapiv1.Kind{resource.KindTCPRoute, resource.KindTLSRoute}
52+
case gwapiv1.TCPProtocolType:
53+
return []gwapiv1.Kind{resource.KindTCPRoute}
54+
case gwapiv1.UDPProtocolType:
55+
return []gwapiv1.Kind{resource.KindUDPRoute}
56+
default:
57+
return nil
58+
}
59+
}
60+
61+
func (t *Translator) validateListenerSpec(listener *ListenerContext, resources *resource.Resources) bool {
62+
// Validate listener spec directly without relying on conditions.
63+
// Start with valid assumption and invalidate on failures.
64+
// Phase 1: Validate fundamental rules
65+
specValid := t.validateAllowedNamespaces(listener)
66+
67+
// Phase 2: Validate allowed routes based on protocol
68+
if isSupportedListenerProtocol(listener.Protocol) {
69+
var tlsMode *gwapiv1.TLSModeType
70+
if listener.TLS != nil {
71+
tlsMode = listener.TLS.Mode
72+
}
73+
allowedKinds := allowedRouteKindsForProtocol(listener.Protocol, tlsMode)
74+
if !t.validateAllowedRoutes(listener, allowedKinds...) {
75+
specValid = false
76+
}
77+
} else {
78+
// Unsupported protocol
79+
specValid = false
80+
listener.SetSupportedKinds()
81+
listener.SetCondition(
82+
gwapiv1.ListenerConditionAccepted,
83+
metav1.ConditionFalse,
84+
gwapiv1.ListenerReasonUnsupportedProtocol,
85+
fmt.Sprintf("Protocol %s is unsupported, must be %s, %s, %s or %s.", listener.Protocol,
86+
gwapiv1.HTTPProtocolType, gwapiv1.HTTPSProtocolType, gwapiv1.TCPProtocolType, gwapiv1.UDPProtocolType),
87+
)
88+
}
89+
90+
// Phase 3: Validate TLS configuration details
91+
if !t.validateTLSConfiguration(listener, resources) {
92+
specValid = false
93+
}
94+
95+
// Phase 4: Validate Hostname configuration
96+
if !t.validateHostName(listener) {
97+
specValid = false
98+
}
99+
100+
return specValid
101+
}
102+
41103
func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource.XdsIRMap, infraIR resource.InfraIRMap, resources *resource.Resources) {
42104
// Infra IR proxy ports must be unique.
43105
foundPorts := make(map[string][]*protocolPort)
106+
107+
// Phase 1: Validate each listener's spec independently.
108+
// This must happen before conflict resolution so that invalid listeners
109+
// don't block valid ones during conflict detection.
110+
for _, gateway := range gateways {
111+
for _, listener := range gateway.listeners {
112+
listener.specValid = t.validateListenerSpec(listener, resources)
113+
}
114+
}
115+
116+
// Phase 2: Run conflict detection.
117+
// Only listeners that haven't been marked as invalid will participate in conflict resolution.
118+
t.validateConflictedProtocolsListeners(gateways)
44119
t.validateConflictedLayer7Listeners(gateways)
45120
t.validateConflictedLayer4Listeners(gateways, gwapiv1.TCPProtocolType)
46121
t.validateConflictedLayer4Listeners(gateways, gwapiv1.UDPProtocolType)
47122
if t.MergeGateways {
48123
t.validateConflictedMergedListeners(gateways)
49124
}
50125

51-
// Iterate through all listeners to validate spec
52-
// and compute status for each, and add valid ones
53-
// to the Xds IR.
126+
// Phase 3: Build IR for valid listeners.
54127
for _, gateway := range gateways {
55128
irKey := t.getIRKey(gateway.Gateway)
56129

@@ -61,50 +134,10 @@ func (t *Translator) ProcessListeners(gateways []*GatewayContext, xdsIR resource
61134
t.processProxyObservability(gateway, xdsIR[irKey], infraIR[irKey].Proxy, resources)
62135

63136
for _, listener := range gateway.listeners {
64-
// Process protocol & supported kinds
65-
switch listener.Protocol {
66-
case gwapiv1.TLSProtocolType:
67-
if listener.TLS != nil {
68-
switch *listener.TLS.Mode {
69-
case gwapiv1.TLSModePassthrough:
70-
t.validateAllowedRoutes(listener, resource.KindTLSRoute)
71-
case gwapiv1.TLSModeTerminate:
72-
t.validateAllowedRoutes(listener, resource.KindTCPRoute, resource.KindTLSRoute)
73-
default:
74-
t.validateAllowedRoutes(listener, resource.KindTCPRoute, resource.KindTLSRoute)
75-
}
76-
} else {
77-
t.validateAllowedRoutes(listener, resource.KindTCPRoute, resource.KindTLSRoute)
78-
}
79-
case gwapiv1.HTTPProtocolType, gwapiv1.HTTPSProtocolType:
80-
t.validateAllowedRoutes(listener, resource.KindHTTPRoute, resource.KindGRPCRoute)
81-
case gwapiv1.TCPProtocolType:
82-
t.validateAllowedRoutes(listener, resource.KindTCPRoute)
83-
case gwapiv1.UDPProtocolType:
84-
t.validateAllowedRoutes(listener, resource.KindUDPRoute)
85-
default:
86-
listener.SetSupportedKinds()
87-
listener.SetCondition(
88-
gwapiv1.ListenerConditionAccepted,
89-
metav1.ConditionFalse,
90-
gwapiv1.ListenerReasonUnsupportedProtocol,
91-
fmt.Sprintf("Protocol %s is unsupported, must be %s, %s, %s or %s.", listener.Protocol,
92-
gwapiv1.HTTPProtocolType, gwapiv1.HTTPSProtocolType, gwapiv1.TCPProtocolType, gwapiv1.UDPProtocolType),
93-
)
94-
}
95-
96-
// Validate allowed namespaces
97-
t.validateAllowedNamespaces(listener)
98-
99-
// Process TLS configuration
100-
t.validateTLSConfiguration(listener, resources)
101-
102-
// Process Hostname configuration
103-
t.validateHostName(listener)
104-
105-
// Process conditions and check if the listener is ready
106-
isReady := t.validateListenerConditions(listener)
107-
if !isReady {
137+
// Finalize listener conditions and check readiness.
138+
t.validateListenerConditions(listener)
139+
if !listener.IsReady() {
140+
// Skip invalid listeners from IR building
108141
continue
109142
}
110143

@@ -243,10 +276,18 @@ func checkOverlappingHostnames(httpsListeners []*ListenerContext) {
243276
if overlappingListeners[i] != nil {
244277
continue
245278
}
279+
// Skip listeners that are already marked as invalid from per-listener validation
280+
if hasInvalidCondition(httpsListeners[i]) {
281+
continue
282+
}
246283
for j := i + 1; j < len(httpsListeners); j++ {
247284
if overlappingListeners[j] != nil {
248285
continue
249286
}
287+
// Skip listeners that are already marked as invalid from per-listener validation
288+
if hasInvalidCondition(httpsListeners[j]) {
289+
continue
290+
}
250291
if httpsListeners[i].Port != httpsListeners[j].Port {
251292
continue
252293
}
@@ -325,10 +366,18 @@ func checkOverlappingCertificates(httpsListeners []*ListenerContext) {
325366
if overlappingListeners[i] != nil {
326367
continue
327368
}
369+
// Skip listeners that are already marked as invalid from per-listener validation
370+
if hasInvalidCondition(httpsListeners[i]) {
371+
continue
372+
}
328373
for j := i + 1; j < len(httpsListeners); j++ {
329374
if overlappingListeners[j] != nil {
330375
continue
331376
}
377+
// Skip listeners that are already marked as invalid from per-listener validation
378+
if hasInvalidCondition(httpsListeners[j]) {
379+
continue
380+
}
332381
if httpsListeners[i].Port != httpsListeners[j].Port {
333382
continue
334383
}

‎internal/gatewayapi/route.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2271,7 +2271,7 @@ func (t *Translator) processAllowedListenersForParentRefs(
22712271
listener.IncrementAttachedRoutes()
22722272
}
22732273

2274-
if !HasReadyListener(selectedListeners) {
2274+
if !HasReadyListener(allowedListeners) {
22752275
routeStatus := GetRouteStatus(routeContext)
22762276
status.SetRouteStatusCondition(routeStatus,
22772277
parentRefCtx.routeParentStatusIdx,

‎internal/gatewayapi/testdata/gateway-with-conflicting-listeners-one-invalid-ref.in.yaml‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
gateways:
2+
- apiVersion: gateway.networking.k8s.io/v1
3+
kind: Gateway
4+
metadata:
5+
namespace: envoy-gateway
6+
name: gateway-1
7+
spec:
8+
gatewayClassName: envoy-gateway-class
9+
listeners:
10+
# This listener has an invalid certificate reference (secret does not exist)
11+
# It should have ResolvedRefs=False with InvalidCertificateRef
12+
# and should be skipped during conflict resolution
13+
- name: invalid-https
14+
protocol: HTTPS
15+
port: 443
16+
hostname: "*.example.com"
17+
allowedRoutes:
18+
namespaces:
19+
from: All
20+
tls:
21+
mode: Terminate
22+
certificateRefs:
23+
- name: non-existent-secret
24+
# This listener has valid configuration and should be accepted
25+
# even though it's on the same port/hostname as the invalid listener
26+
- name: valid-https
27+
protocol: HTTPS
28+
port: 443
29+
hostname: "*.example.com"
30+
allowedRoutes:
31+
namespaces:
32+
from: All
33+
tls:
34+
mode: Terminate
35+
certificateRefs:
36+
- name: tls-secret-1
37+
secrets:
38+
- apiVersion: v1
39+
kind: Secret
40+
metadata:
41+
namespace: envoy-gateway
42+
name: tls-secret-1
43+
type: kubernetes.io/tls
44+
data:
45+
tls.crt: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0tCk1JSUR3RENDQXFpZ0F3SUJBZ0lVVDRyelIreStHd1VzMm9ydExIZ0k1MzBKeG9Fd0RRWUpLb1pJaHZjTkFRRUwKQlFBd0xURVZNQk1HQTFVRUNnd01aWGhoYlhCc1pTQkpibU11TVJRd0VnWURWUVFEREF0bGVHRnRjR3hsTG1OdgpiVEFlRncweU5UQTBNakl3TWpVNU1UQmFGdzB6TlRBME1qQXdNalU1TVRCYU1EVXhGREFTQmdOVkJBTU1DMlp2CmJ5NWlZWEl1WTI5dE1SMHdHd1lEVlFRS0RCUmxlR0Z0Y0d4bElHOXlaMkZ1YVhwaGRHbHZiakNDQVNJd0RRWUoKS29aSWh2Y05BUUVCQlFBRGdnRVBBRENDQVFvQ2dnRUJBTE4wbnJNR1NZNjBPT0JuTFVaSGpCRFkxazhqWHA2RwppeG1RaFNOK3lZUi9VQWVqSmhCOVI3S2RuT3d0eGljTnozdUFtL0p0UTFKRUU0dW5xQnhVTU8ydWpvVXl4ZisrCjRnb2tFYmVpTlhNN0ptaklPOGxEWlJjcEhFTlE3eUNJL3d1cEZBcHgwNnVrNUtBSlpRMUlmVXhZWS9RRkJsc3cKdUx0TWozVlB6eDBJYjRIV1lHdGhXcDIzUWduMUdGUWVTOGMwZHdqWWNBTGtrTFdwWWdUZTZGT0VhR3hvUzdwTQpGbitvZnRFUjlxZCtDcnRDSXM5TzFtOW5MUU9UWGJDNU5nSzR1ZFdhMFBuYjJ6TEk4WjZsVHFRSTNSODE2NkxKCkVDQi9ZSlYzVmtMb2cxUmx0d0FrM3hrWHFnbVhTZUxILzY3MHh6MEx5OGZHQVpmejFMQVpkSXNDQXdFQUFhT0IKenpDQnpEQWRCZ05WSFE0RUZnUVUzbVFodVB2dVl1K0lmN0ZaM010eU9jMWdjQ1V3YUFZRFZSMGpCR0V3WDRBVQpXWmxKWFQ1bXlEVnlsUjlSS2JQQTAxTkVlcytoTWFRdk1DMHhGVEFUQmdOVkJBb01ER1Y0WVcxd2JHVWdTVzVqCkxqRVVNQklHQTFVRUF3d0xaWGhoYlhCc1pTNWpiMjJDRkJuNktuTlBhbm1Db1daVStNYmtwKzJScmN4dU1Bc0cKQTFVZER3UUVBd0lDL0RBcEJnTlZIUkVFSWpBZ2dnOW1iMjh1WlhoaGJYQnNaUzVqYjIyQ0RTb3VaWGhoYlhCcwpaUzVqYjIwd0NRWURWUjBTQkFJd0FEQU5CZ2txaGtpRzl3MEJBUXNGQUFPQ0FRRUFIa2xEbzkvNnRLcDNFd3JSCnJjVStKOUtmUkFGajc5YU1DREpVb0NyM2J6RFIycXQ4ZzlsbDdFSzZaeEtFa0xzWkFlYzBxU0Z1QjBvbzVqZFEKM3VvT2hNK1JKTkZoTldFd3dHWmpMb0FlK2oxaXByN1A5ajdmdFNzck8ra3M3TVNMeTE2RW9IV2Q0eG00Rk5QZQpmUVpRYWhpTTdMYVFCdW5wdXlhZWtLdG5tU241RzlkSHpGeTVNelRSbFJyVWxhVzdVbDRUeExlOEROZ2ZpR2ZCCnpjcmpVK2l3RUJXeS94b3B2aDEzNmlybmV3NTg3RWt3dzQ3QXFhc3gvZStMK2NhSlYySGVMd0dSaE5xR2pIcjkKQ2dSVHpud3F5QjdtTVNXYStWaXBNSHlUVmRCNjRvYjZxbkdqTldvWXdRbUUraU0vL0VrTURzd2JVcTc2R1pKMApsWlRkTlE9PQotLS0tLUVORCBDRVJUSUZJQ0FURS0tLS0tCg==
46+
tls.key: LS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tCk1JSUV2UUlCQURBTkJna3Foa2lHOXcwQkFRRUZBQVNDQktjd2dnU2pBZ0VBQW9JQkFRQ3pkSjZ6QmttT3REamcKWnkxR1I0d1EyTlpQSTE2ZWhvc1prSVVqZnNtRWYxQUhveVlRZlVleW5aenNMY1luRGM5N2dKdnliVU5TUkJPTApwNmdjVkREdHJvNkZNc1gvdnVJS0pCRzNvalZ6T3lab3lEdkpRMlVYS1J4RFVPOGdpUDhMcVJRS2NkT3JwT1NnCkNXVU5TSDFNV0dQMEJRWmJNTGk3VEk5MVQ4OGRDRytCMW1CcllWcWR0MElKOVJoVUhrdkhOSGNJMkhBQzVKQzEKcVdJRTN1aFRoR2hzYUV1NlRCWi9xSDdSRWZhbmZncTdRaUxQVHRadlp5MERrMTJ3dVRZQ3VMblZtdEQ1MjlzeQp5UEdlcFU2a0NOMGZOZXVpeVJBZ2YyQ1ZkMVpDNklOVVpiY0FKTjhaRjZvSmwwbml4Lyt1OU1jOUM4dkh4Z0dYCjg5U3dHWFNMQWdNQkFBRUNnZ0VBSmYrYW53UEV6WS9CdjFwNWpya1ZvbmVYb1hnMno5QmpZYzFsTTZma0djY3YKZGY2SXo5TUhQSDM5UFZGUDlQTUtyUGNGam1hdWE1djRtNGlyb3h2OHBFZGk3RGRkRDVNbW44a1ZhMUhRaVk3TAp5a0lqenJFVGxiemh2Q3RHQnhpYkVLZ0RrMWFZNEc1dzdxWXVuSXB0NVoyTnhKelB4TDFqVUYyY3Z0VmdZS0FPCjN2TWY1dENWbS9kL2JBT2xvWnVVc3ArSnc1ZTYvSFN3c3lEdjV2Y2dDQkRXRjFkTFFiYUxCelZDZGJBY0Fma1AKbFUzeGhZMVYyMGU2NmhBbU0veE53MzArSUhvTUlTUTFiQW9MOXVzN3NsTlYxT2ZNZk5hWC83WVdKWFVab1MzNwpyY1IvVXYwZlRtQkhPR3pwK25oaWpLRUlYU1ZESjNxK040amM4Ni9IeVFLQmdRRFhWbzNKSmlia0cvY1JBZ2pICkVqdWROMHJMSGJvVldPanN5ekZmaXg1MHJKTTYwMDJKZURicGdQeEt4ZUtFdGl4ZkVWQmU0eHdqNG9oK1ZubEIKT0dMU3BWcHRRVFRnRVprWXFvVU1iUDIyNHQ2cUZuSXZsTkkwZFJGWDUyS3I2L1FCS0EwWEk3K0I1bTV3Z2hLcgo4ZEQrMGRWNkZzMmxFT0NMWUJKck1weFNIUUtCZ1FEVlY0REVaUlFvVUo5UGRkb3IyK2IweFV2Y3JHeUJ1NlM2CjJZOFJuMzdWUVlBSThHUnlYRkcxTGxZaHpMOWNwd0tUR21SMVhPRTVkMC9mMStaU3h5TVc1d2FSaXU1b0dFK2cKUlNsMXBBdTNUdlEzM1BVNUJzNFhhcHBLOC9BZVk3cG1pd1JhYnRPSFlQa0FHMXNzL3k1d1NjVGUxQXZiWlpsTAo1TmgwdHZvZ3h3S0JnUUNtRkFJOFhlbG15czZ0Vm1WUXE1WkF0YkZBb0VleFNTWXo0cTdNb200MXpCZXRLZVRHCkhtb3pneUNSeHJiaVplSW8zQ0NoWGdXSkE2RUQxMHVqYW9xRkxiUmxTUUl2d2tMU1RFbGJBUUJZdWZiRE5aYVIKYmZVRk1qalRGQWo4MFhrYUh6cWhXeGZMWnQ1TWRYVlRHYWgzcjN3MnNqbWVranFzSThkdzE5TEtYUUtCZ0NoMQp3T0QrUG5WcTNOdklBUWxpV2dtL3hTUmp1dXhidHVFTTA1cEhBbG5WWXovT3YyNEUzaVliVkpCeWNUUlVKQ1BiCjFJT0JpdUZJSkdqU1hFY0VwejMzc0lJM3RBRWY0eklGQzlqWXRMUWVFQ2pzQ2NHMzdhdjVOcXZTV1k2WjRVY0QKUkY4V041MnNJVzBJd3lEa2dGMGhVR25tRXgyWHhodmptYjJBMmkwUEFvR0FUV1kzbXZRVWtZcmU5R0J0alVqYgpDWUx5Mm1VVElKbjVDcmhjTHMvMlh5MkNoVSt5MnhsT3pIcm1vanRIaHFFcnVCK0xyZkpBd3B3cGVRNmxyWnFQCjFGazVKT2c2bmV2U3lDcWF4SDZaenZRMXJ6VzAycm1IMUJrV1BiY0NwKy9VRTIrS3JselZtNHkvZ0ZoRGVGZlcKRUc5UzIzYndKME1JUmF0WVpFQm0wcWs9Ci0tLS0tRU5EIFBSSVZBVEUgS0VZLS0tLS0K
47+
httpRoutes:
48+
- apiVersion: gateway.networking.k8s.io/v1
49+
kind: HTTPRoute
50+
metadata:
51+
namespace: default
52+
name: httproute-1
53+
spec:
54+
parentRefs:
55+
- namespace: envoy-gateway
56+
name: gateway-1
57+
sectionName: valid-https
58+
rules:
59+
- matches:
60+
- path:
61+
value: "/"
62+
backendRefs:
63+
- name: service-1
64+
port: 8080

0 commit comments

Comments
 (0)