-
Notifications
You must be signed in to change notification settings - Fork 480
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
Clarify Gateway Listener Status AttachedRoutes field #2396
Clarify Gateway Listener Status AttachedRoutes field #2396
Conversation
Document that the field intends to count purely Gateway<->Route relationships regardless of validity of either resource. Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
/approve Nice work, thanks @sunjayBhatia! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: shaneutt, sunjayBhatia, youngnick The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
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 <s.berman@f5.com>
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 <s.berman@f5.com>
What type of PR is this?
/kind cleanup
/kind documentation
What this PR does / why we need it:
Document that the field intends to count purely Gateway<->Route relationships regardless of validity of either resource.
Which issue(s) this PR fixes:
Fixes #1626
Does this PR introduce a user-facing change?: