-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conform to Gateway spec for AttachedRoutes of Gateway #1148
Labels
bug
Something isn't working
refined
Requirements are refined and the issue is ready to be implemented.
size/medium
Estimated to be completed within a week
Milestone
Comments
Closed
6 tasks
ja20222
added
size/medium
Estimated to be completed within a week
refined
Requirements are refined and the issue is ready to be implemented.
labels
Nov 7, 2023
6 tasks
pleshakov
added a commit
to pleshakov/nginx-gateway-fabric
that referenced
this issue
Nov 27, 2023
Problem: - Rules for traffic attachment of HTTPRoutes to Gateways were clarified in kubernetes-sigs/gateway-api#2396 -- successful attachment should depend only on parentRefs in an HTTPRoute and AllowedRoutes of a Listener, even if either or both of them are invalid. - The corresponding conformance test GatewayWithAttachedRoutes was added in kubernetes-sigs/gateway-api#2477 , which fails for NGINX Gateway Fabric. - NGINX Gateway Fabric will not try to attach an HTTPRoute to a Listener if either or both of them are invalid. Solution: - Make NGF compliant with the Gateway API and make the corresponding test pass. - Introduce Attachable fields for Listener and HTTPRoute types of the Graph in the graph package. - Update the validation logic: - NGF considers a Listener attachable if (a) its hostname is valid, (b) protocol is supported by NGF and (c) AllowedRoutes are valid. - NGF considers an HTTPRoute attachable if (d) its hostnames are valid. - Attach an HTTPRoute to a Listener if both are attachable. Note: (a), (b) and (d) are not mentioned in kubernetes-sigs/gateway-api#2396 However, they are necessary: For (b), NGF doesn't know how to attach to non-supported protocols like TCP. For (a), Listener hostname needed for HTTPRoute attaching, because it affects if an HTTPRoute can attach or not (per Gateway API spec). For (c), HTTPRoute hostnames are also needed, because they affect if an HTTPRoute can attach or not per Gateway API spec). See https://github.com/kubernetes-sigs/gateway-api/blob/52c2994ed9de1c287a37465490b91cfcf01bf16e/apis/v1/httproute_types.go#L71-L73 Testing: - Unit tests are updated and extended - Failing conformance test GatewayWithAttachedRoutes now passes. CLOSES nginxinc#1148 Co-authored-by: Saylor Berman <[email protected]>
github-project-automation
bot
moved this from 👀 In Review
to ✅ Done
in NGINX Gateway Fabric
Nov 27, 2023
pleshakov
added a commit
that referenced
this issue
Nov 27, 2023
Problem: - Rules for traffic attachment of HTTPRoutes to Gateways were clarified in kubernetes-sigs/gateway-api#2396 -- successful attachment should depend only on parentRefs in an HTTPRoute and AllowedRoutes of a Listener, even if either or both of them are invalid. - The corresponding conformance test GatewayWithAttachedRoutes was added in kubernetes-sigs/gateway-api#2477 , which fails for NGINX Gateway Fabric. - NGINX Gateway Fabric will not try to attach an HTTPRoute to a Listener if either or both of them are invalid. Solution: - Make NGF compliant with the Gateway API and make the corresponding test pass. - Introduce Attachable fields for Listener and HTTPRoute types of the Graph in the graph package. - Update the validation logic: - NGF considers a Listener attachable if (a) its hostname is valid, (b) protocol is supported by NGF and (c) AllowedRoutes are valid. - NGF considers an HTTPRoute attachable if (d) its hostnames are valid. - Attach an HTTPRoute to a Listener if both are attachable. Note: (a), (b) and (d) are not mentioned in kubernetes-sigs/gateway-api#2396 However, they are necessary: For (b), NGF doesn't know how to attach to non-supported protocols like TCP. For (a), Listener hostname needed for HTTPRoute attaching, because it affects if an HTTPRoute can attach or not (per Gateway API spec). For (c), HTTPRoute hostnames are also needed, because they affect if an HTTPRoute can attach or not per Gateway API spec). See https://github.com/kubernetes-sigs/gateway-api/blob/52c2994ed9de1c287a37465490b91cfcf01bf16e/apis/v1/httproute_types.go#L71-L73 Testing: - Unit tests are updated and extended - Failing conformance test GatewayWithAttachedRoutes now passes. CLOSES #1148 Co-authored-by: Saylor Berman <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
refined
Requirements are refined and the issue is ready to be implemented.
size/medium
Estimated to be completed within a week
In kubernetes-sigs/gateway-api#2396, it was clarified that the Gateway Listener status AttachedRoutes field is a count of the number of Routes associated with a Listener regardless of Gateway or Route status.
NGF:
nginx-gateway-fabric/internal/mode/static/state/graph/httproute.go
Line 328 in 6707e5a
nginx-gateway-fabric/internal/mode/static/state/graph/httproute.go
Line 266 in 6707e5a
Acceptance criteria:
See also kubernetes-sigs/gateway-api#2402 (conformance tests for clarified behaviour)
The text was updated successfully, but these errors were encountered: