Expand and clarify Listeners definition #2288
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -85,38 +85,55 @@ type GatewaySpec struct { | ||
| // | ||
| // Port and protocol combinations not listed above are considered Extended. | ||
| // | ||
| // A Gateway's Listeners are considered "compatible" if: | ||
| // | ||
| // 1. The implementation can serve them in compliance with the Addresses | ||
| // requirement that all Listeners are available on all assigned | ||
| // addresses. | ||
| // 2. The implementation can match inbound requests to a single distinct | ||
| // Listener. When multiple Listeners share values for fields (for | ||
| // example, two Listeners with the same Port value), the implementation | ||
| // can can match requests to only one of the Listeners using other | ||
| // Listener fields. | ||
| // | ||
| // Compatible combinations in Extended support are expected to vary across | ||
| // implementations. A combination that is compatible for one implementation | ||
| // may not be compatible for another. | ||
| // | ||
| // If this field specifies multiple Listeners that are not compatible, the | ||
| // implementation MUST set the "Conflicted" condition in the Listener | ||
| // Status to "True". | ||
| // | ||
| // Implementations MAY choose to still accept a Gateway with conflicted | ||
| // Listeners if they accept a partial Listener set that contains no | ||
| // incompatible Listeners. They MUST set a "ListenersNotValid" condition | ||
| // the Gateway Status when the Gateway contains incompatible Listeners | ||
| // whether or not they accept the Gateway. | ||
|
Comment on lines
+107
to
+111
There was a problem hiding this comment. I think the only reason we'd want to allow this is if a Gateway was already programmed with compatible listeners and then invalid/incompatible one(s) were added. It gets really tricky to represent this state. I may be remembering wrong here, but I think on GKE we will leave "Programmed" set to "True" with the last generation it was Programmed, but I don't think we have any clear guidance for these kinds of partially valid states in the spec. There was a problem hiding this comment. |
||
| // | ||
| // For example, the following Listener scenarios may be compatible | ||
| // depending on implementation capabilities: | ||
| // | ||
| // 1. Multiple Listeners with the same Port that all use the "HTTP" | ||
|
There was a problem hiding this comment. this example is a core feature I wonder if "may be compatible" is applicable to itThere was a problem hiding this comment. Compatibility is weird because it ends up being about implementation capabilities rather than rules in the spec. If your implementation isn't compatible with something the core requires, it just can't implement the spec. We could bring this further out of core by making it something outside the language above and mentioning specific ports, e.g. "Multiple Listeners with Port "9999" that all use the "HTTP" protocol (and similar for the other examples, just to make them consistent). We could also separate examples that are within core or outside it, but it wouldn't be great for brevity. |
||
| // Protocol that all have unique Hostname values. | ||
|
There was a problem hiding this comment. I know this is just an example, so it may not be critical here, but it might be helpful to define whether There was a problem hiding this comment. 8a9acfb adds this to the following Hostname precedence section. Do you think that works? It's not explicitly saying they are considered distinct, but it is using them as distinct values in the context where it matters. |
||
| // 2. Multiple Listeners with the same Port that use either the "HTTPS" or | ||
| // "TLS" Protocol that all have unique Hostname values. | ||
| // 3. A mixture of "TCP" and "UDP" Protocol Listeners, where no Listener | ||
| // with the same Protocol has the same Port value. | ||
| // | ||
| // An implementation that cannot serve both TCP and UDP listeners on the same | ||
| // address, or cannot mix HTTPS and generic TLS listens on the same port | ||
| // would not consider those cases compatible. | ||
| // | ||
| // Implementations using the Hostname value to select between same-Port | ||
| // Listeners MUST match inbound request hostnames from the most specific | ||
| // to least specific Hostname values to find the correct set of Routes. | ||
| // Exact matches must be processed before wildcard matches, and wildcard | ||
| // matches must be processed before fallback (empty Hostname value) | ||
| // matches. For example, `"foo.example.com"` takes precedence over | ||
| // `"*.example.com"`, and `"*.example.com"` takes precedence over `""`. | ||
| // | ||
| // Implementations MAY merge separate Gateways onto a single set of | ||
| // Addresses if all Listeners across all Gateways are compatible. | ||
| // | ||
| // Support: Core | ||
| // | ||
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.
Nit: ListenersNotValid is a reason that can be used with "Accepted", generally to set the condition to "False". I'm not sure what we'd recommend in the case where Listeners were not valid and the Gateway was accepted.
Uh oh!
There was an error while loading. Please reload this page.
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.
We say it can be used when Accepted is True. From the first point in my self-review, this is indeed a confusing case, but probably one that's intentionally ambiguous because we expect it to vary so much between implementations.
If we want to leave that as-is, the change here is just to say (here) that implementations must set this, whereas previously it wasn't a strict requirement, and wasn't obvious unless you looked through the reason comments. Even if it changes, I think we should mention something here.
Changing the current vague guidelines to something more formal would be a significant breaking change. Between that and the expected variance across vendors, my vote would be to defer it to post GA, when we have more practical experience regarding which approaches are in use and their pros and cons.