[Possible breaking change] - Making listeners optional on Gateway - Needs sign off

[rikatz]

What

Based on ListenerSet promotion, we want to make Gateway listeners optional starting on release 1.5.

This means that users now may be able to create a Gateway without Listeners as soon as they have the "allowedListeners" field defined.

Why

ListenerSet specification makes gateway listeners optional. We want to make this change sooner than later, and right now enforcing Gateway admins to set a listener while allowing users to attach ListenerSet may cause undesired behaviors, and force admins to create dummy listeners that can become a problem later

How

This change will:

• Create a validation on Gateway's spec.listeners that would make it required IF spec.allowedListeners is not null
• Drop the minItems from Gateway spec.listeners

What we need from you?

• If you are someone that represents an implementation, please verify if your implementation will not break with this change (aka you check the listeners array) and SIGN OFF on this issue saying you are aware / ok with it and the implementation name
• If you are someone that represents an implementation and disagree with this change right now, please provide some more information on how this may impact you.
• If you are a concerned user, please add your concerns

[rikatz]

I will ping some people that I know that may represent an implementation based on latest iterations or conformance tests submissions

@howardjohn @keithmattix @snorwin @arkodg @davidjumani @sjberman @youngnick @kevinpollet @jcmoraisjr

[sjberman]

For NGINX Gateway Fabric, this does break us, but it can be fixed.

When a Gateway is created, we provision the LB Service with the ports specified in the listeners. If there are no listeners, the Service fails to create because ports are required.

We'll need to update our controller to skip provisioning if there are no listeners.

The reason I don't think this is an issue for us is that each of our releases supports a specific Gateway API version, and we don't guarantee future compatibility. So our most recent release supports 1.4; if you upgrade to 1.5 and remove your listeners without installing an NGF version that supports 1.5, then things will break for you.

[howardjohn]

We'll need to update our controller to skip provisioning if there are no listeners.

This may be problematic actually since you are supposed to report an address in the status, right? Also, as a user, I would be surprised to find my first listener (when I don't know its my first) random takes minutes to provision since it accidentally was the first. And then I delete it, and my LB gets torn down, and the IP changes.

Its quite a bummer there is no way to make a Service with no ports. In istio we accidentally work around this by exposing a fixed port in the service always, but some users turn that off so we would break in the same way.

So I would say, for implementations I involved in:

• Istio: would break some users
• Kgateway: would break
• Agentgateway: would break

[zac-nixon]

This would break our implementation (https://github.com/kubernetes-sigs/aws-load-balancer-controller) but is a rather easy fix.

[snorwin]

As already mentioned during the community meeting, from the Airlock side we don’t expect any issues. We only support existing versions, and the controller stops reconciling if it encounters a CRD version newer than the one it was released with.

[rikatz]

I am looking here into alternatives and wanted to discuss:

• If we postpone to 1.6, will implementations be able to fix it? Per my understanding on @howardjohn comment the problem is not just about making it optional, but the fact that the listeners are used to provision services (as they will be the real exposed ports) and we are deferring it to users.
• If not, adding a dummy listener would need to be part of the documentation and really clear for gateway admins that the one would be a dummy listener, that must have something like a weird domain (well, this may break external-dns...) or have some restrictions like not allowing anyone to attach to it (so it would be used just to provision a port). Like gateway admins must add a dummy listener with no hostname (does it become a catch all in that case?) and with an invalid namespace selector (like a label that will never ever be selectable).

[howardjohn]

If we postpone to 1.6, will implementations be able to fix it

TBH not sure what the 'fix' even would be. Assuming an impl with an in-cluster LoadBalancer Service... just make a Service with a bogus port we reserve?

It would work but fwiw we have had users complain that we opened ports they didn't explicitly approve

[robscott]

TBH not sure what the 'fix' even would be. Assuming an impl with an in-cluster LoadBalancer Service... just make a Service with a bogus port we reserve?

This is a great question @howardjohn! At a minimum, I think the fix we're looking for here is "not crash". It would be useful to define some more common patterns though:

1) ListenerSets are supported, but no Listeners are defined on Gateway or attached ListenerSets
I'll defer to ListenerSet experts on this one, I'm not really sure what the standard should be - cc @dprotaso @davidjumani @rikatz

2) ListenerSets are not supported, but no Listeners are defined on the Gateway
Since the CEL validation would only allow listeners to be empty if the user had configured allowedListenerSets, the user is clearly expecting behavior that the implementation does not support. In this case, I'd expect the controller to reject the Gateway with Accepted: False and a message describing that ListenerSets are not supported and therefore a Gateway must specify at least one Listener.

[rikatz]

I'd expect the controller to reject the Gateway with Accepted: False and a message describing that ListenerSets are not supported and therefore a Gateway must specify at least one Listener.

I think the problem here is about version drift. In case the controller has an older library that never supported the allowedListenerSets field, it will not be able to verify it. So a user that installs GW API CRD 1.5 but the controller relies on, let's say, 1.1 (or some that didn't had the allowedListenerSets) or even migration between versions (as this field is promoted to standard on 1.5 and will "appear") will never know about the existence of this field, and rather cannot reject something it doesn't know that exists

[rikatz]

For the first problem, on John's point, I think there are some concerns here (at least I can see some during gateway creation period):

• Given some cloud providers create a LoadBalancer based on a service, and some (if not the majority, but not all) of the implementations create a service from a Gateway with a listener, once a Gateway is being created is usually also the moment a LoadBalancer is being deployed. In this case we cannot guarantee that a Gateway will have a listener that specifies "how a service should be created" as there will not be ports allocated, but also we do not have yet ListenerSets as they will be attached to a Gateway (a user may create a ListenerSet upfront before the Gateway, but this is IMO a weird workflow for a creation process)
• Still on this case, what would happen in case a user creates a Gateway and there are already ListenerSets defined attaching to this "to exist" Gateway is a question. Or if the selector for allowedListenerSet is set, but wrong? We will have a Gateway with no Listeners and no ListenerSets.

Probably a service should be created just once a controller can guarantee that a Gateway has at least one bind port, but I will defer this to implementations to tell me how viable it is, and what UX impact may have

[davidjumani]

Probably a service should be created just once a controller can guarantee that a Gateway has at least one bind port, but I will defer this to implementations to tell me how viable it is, and what UX impact may have

I'm leaning towards this idea, however we would need to define how to handle an invalid gateway (eg: with invalid listeners) vs a gateway without any listeners