-
Couldn't load subscription status.
- Fork 1.6k
[feat gw-api]handle hostname intersection #4417
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
base: main
Are you sure you want to change the base?
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: shuqz The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
fb0854b to
3aed806
Compare
|
/retest |
| GetRouteGeneration() int64 | ||
| GetRouteCreateTimestamp() time.Time | ||
| GetCompatibleHostnames() []gwv1.Hostname | ||
| SetCompatibleHostnames([]gwv1.Hostname) |
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.
It would be great to add a comment saying what a "compatible hostname" is.
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.
From gateway documentation,
Hostnames defines a set of SNI names that should match against the
SNI attribute of TLS ClientHello message in TLS handshake. This matches
the RFC 1123 definition of a hostname with 2 notable exceptions:
1. IPs are not allowed in SNI names per RFC 6066.
2. A hostname may be prefixed with a wildcard label (*.). The wildcard
label must appear by itself as the first label.
If a hostname is specified by both the Listener and TLSRoute, there
must be at least one intersecting hostname for the TLSRoute to be
attached to the Listener. For example:
A Listener with test.example.com as the hostname matches TLSRoutes
that have either not specified any hostnames, or have specified at
least one of test.example.com or *.example.com.
A Listener with *.example.com as the hostname matches TLSRoutes
that have either not specified any hostnames or have specified at least
one hostname that matches the Listener hostname. For example,
test.example.com and *.example.com would both match. On the other
hand, example.com and test.example.net would not match.
If both the Listener and TLSRoute have specified hostnames, any
TLSRoute hostnames that do not match the Listener hostname MUST be
ignored. For example, if a Listener specified *.example.com, and the
TLSRoute specified test.example.com and test.example.net,
test.example.net must not be considered for a match.
If both the Listener and TLSRoute have specified hostnames, and none
match with the criteria above, then the TLSRoute is not accepted. The
implementation must raise an 'Accepted' Condition with a status of
False in the corresponding RouteParentStatus.
| route *gwv1.HTTPRoute | ||
| rules []RouteRule | ||
| ruleAccumulator attachedRuleAccumulator[gwv1.HTTPRouteRule] | ||
| compatibleHostnames []gwv1.Hostname |
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.
How does this work for a route that attaches to two different listeners in a Gateway?
| // A route can attach to listener if it does not have hostname or listener does not have hostname | ||
| if listener.Hostname == nil || len(route.GetHostnames()) == 0 { | ||
| // If route has no hostnames but listener does, use listener hostname | ||
| if len(route.GetHostnames()) == 0 { |
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.
Don't we need a nil check still?
Description
we were not properly handle case when listener has hostname but route does not, therefore failing conformance test - HTTPRouteListenerHostnameMatching
This implementation covers this gap by
Checklist
README.md, or thedocsdirectory)re-run HTTPRouteListenerHostnameMatching and got
BONUS POINTS checklist: complete for good vibes and maybe prizes?! 🤯