validate Gateway listener protocols/ports/hostnames #4462
Conversation
Adds validation to both the Gateway provisioner and the Gateway API DAG processor to check HTTP protocol listeners to ensure a single port, and no hostname conflicts. The provisioner uses this to configure the correct port on the Envoy service, and the DAG processor uses it to set listener conditions and process only valid listeners. Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
Signed-off-by: Steve Kriss <krisss@vmware.com>
Updates the Envoy service provisioning code to only use ports from the listeners, instead of using defaults if not specified in the model. Signed-off-by: Steve Kriss <krisss@vmware.com>
Codecov Report
@@ Coverage Diff @@
## main #4462 +/- ##
==========================================
+ Coverage 76.48% 76.65% +0.16%
==========================================
Files 138 139 +1
Lines 12433 12490 +57
==========================================
+ Hits 9510 9574 +64
+ Misses 2674 2669 -5
+ Partials 249 247 -2
|
skriss
added
the
release-note/small
|
Looking pretty good in manual testing as well, marking ready for review. |
skriss
requested review from
tsaarni,
stevesloka,
sunjayBhatia and
youngnick
and removed request for
a team,
tsaarni and
stevesloka
| } | ||
|
|
||
| // All listeners with a protocol of "HTTP" must use the same port number | ||
| // Heuristic: the first port number encountered is allowed, any other listeners with a different port number are marked "Detached" with "PortUnavailable" |
There was a problem hiding this comment.
this may be something to call out in a doc somewhere, i dont think this specific grouping/heuristic is in the gateway docs themselves (looking at the Listeners field documentation here https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.Gateway)
There was a problem hiding this comment.
Yeah, this is really a Contour-specific thing since we only support 2 ports total right now. Agree we should call this out somewhere - need to figure out where to put docs for all this, but let me add this to a new issue.
Signed-off-by: Steve Kriss <krisss@vmware.com>
|
looks good! simple logic for determining conflicts/choosing a port 👍🏽 seems right that the provisioner will do similar collection of listeners to program an envoy service but contour can do the more detailed validation and set status |
Yeah originally i was thinking they'd both need to set status, which requires more care in terms of ensuring they're not fighting with each other (a la the discussions we were having ~18 months ago), though I think would've been doable given it's all now a single codebase, but realized that it was probably fine to just have Contour itself set the status, the provisioner just needs to find out what ports to configure. We'll see if any issues pop up with that model, but I think it works pretty well actually. |
Ensures that there is at most one insecure
port and one secure port across all Gateway
listeners, and that hostnames are unique
within the groups of insecure and secure
listeners, respectively.
Closes #4439.
Leaving as a draft for now to do some more manual testing + ensure coverage is good, also feel free to poke holes in the model, definitely possible I've misinterpreted something.