Skip to content
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

GEP-1713: Standard Mechanism to Merge Multiple Gateways #1863

Closed
wants to merge 36 commits into from
Closed
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
056c52f
initial commit of GEP 1713
dprotaso Mar 22, 2023
b0da061
Flush out the GEP
dprotaso Mar 28, 2023
3f50cea
fix typos
dprotaso Mar 28, 2023
09b29d1
Accepted should trigger other conditions to be set to 'False'
dprotaso Mar 28, 2023
f4f3928
Drop parent-gateway label
dprotaso Apr 4, 2023
4c5b38b
add clause that child listeners should not appear on the parent
dprotaso Apr 4, 2023
cbfea8c
update example of attaching to a child gateway
dprotaso Apr 4, 2023
832fa8f
adjust invalid scenarios and allow for rejection via webhook
dprotaso Apr 4, 2023
492eff7
flush out listener precedence and conflict semantics
dprotaso Apr 4, 2023
bc245cd
fix header indentation
dprotaso May 4, 2023
951081e
Add a note that a child Gateway can attach only to other Gateways in …
dprotaso May 4, 2023
aa18975
define parent gateway and update correct usage of primary vs parent
dprotaso May 4, 2023
ed2c90b
Define validation semantics on the concatenated list of listeners
dprotaso Aug 2, 2023
ba5a47d
move GEP to dedicated directory
dprotaso Feb 5, 2024
e2331f0
rework GEP with input from howardjohn
dprotaso Feb 6, 2024
e914896
undo removing future goal of making merging a MUST requirement
dprotaso Feb 6, 2024
b3f7113
fix some typos
dprotaso Feb 7, 2024
e094dff
attaching to a Gateway resource must be permitted
dprotaso Feb 7, 2024
dcdd142
allow empty sectionName/port when targetting a child Gateway
dprotaso Feb 12, 2024
668af46
Add TooManyListeners failure mode
dprotaso Feb 12, 2024
ec9ce63
address feedback from sunjay
dprotaso Feb 12, 2024
d3ca6b1
Update geps/gep-1713/index.md
dprotaso Mar 15, 2024
5d6853d
Update geps/gep-1713/index.md
dprotaso Mar 15, 2024
aa2d649
Update geps/gep-1713/index.md
dprotaso Mar 15, 2024
df3b0dc
rename GatewayObjectReference to ParentGatewayReference
dprotaso Jul 3, 2024
6e4e7ab
Include a count of attached child gateways
dprotaso Jul 3, 2024
23f5e15
Include some more alternative solutions
dprotaso Jul 3, 2024
969c5a1
mention route delegation GEP
dprotaso Jul 3, 2024
4e05526
mention use of CEL
dprotaso Jul 3, 2024
a979bca
fix references section
dprotaso Jul 3, 2024
98eca50
include comments why some alternatives were rejected
dprotaso Jul 3, 2024
0de79b6
include a case where two child gateways conflict
dprotaso Jul 3, 2024
9add2e6
change word to mechanism
dprotaso Jul 3, 2024
01cb1fd
Update geps/gep-1713/index.md
dprotaso Jul 3, 2024
df249a5
add numbers to order precedence
dprotaso Jul 3, 2024
43074ed
Update language about gateway conflict and direct them to the gateway…
dprotaso Jul 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
381 changes: 381 additions & 0 deletions geps/gep-1713/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,381 @@
# GEP-1713: Standard Mechanism to Merge Multiple Gateways

* Issue: [#1713](/kubernetes-sigs/gateway-api/issues/1713)
* Status: Provisional

(See status definitions [here](overview.md#status).)

## tl;dr

The Gateway Resource is a contention point since it is the only place to attach listeners with certificates. We propose a mechanism to allow distinct Gateway resources to be logically merged.

## Goals

- Define a mechanic to merge multiple Gateways (logically)
- Define a set of acceptable properties that can be merged and their semantics
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

## Non-Goals

- Apply a Gateway resource onto N distinct gateways (one to many)

## Introduction

Knative generates on demand per-service certificates using HTTP-01 challenges.
There can be O(1000) Knative Services in the cluster which means we have O(1000) distinct certificates.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
Thus updating a single Gateway resource with this many certificates is a contention point and inhibits horizontal scaling of our controllers.
[Istio Ambient](https://istio.io/v1.15/blog/2022/introducing-ambient-mesh/), similarly, creates a listener per Kubernetes service.
Comment on lines +23 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the primary motivation here is to enable use cases that would rely on Listener generation. If that's correct, maybe there's a simpler solution than Gateway merging?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe there's a simpler solution than Gateway merging?

Care to provide some alternative solutions? I'm going to close this out end of week otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree there are lots of other reasons we'd want to specify gateway merging - the main one would be having fewer resources to manage. If multiple listeners shared an endpoint, may as well merge them to reduce traffic as well. If this proposal isn't really motivated by making things more efficient for those reasons, maybe gateway merging isn't the right solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started a doc for brainstorming here: https://docs.google.com/document/d/1qj7Xog2t2fWRuzOeTsWkabUaVeOF7_2t_7appe8EXwA/edit. I've added this proposal + a couple alternatives there, interested in any additional ideas people have. (They all include some form of Gateway merging for the record).


More broadly, large scale gateway users often expose O(1000) domains, but are currently limited by the maximum of 16 `listeners`.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

The spec currently has language to indicate implementations `MAY` merge Gateways resources but the mechanic isn't defined.
https://github.com/kubernetes-sigs/gateway-api/blob/541e9fc2b3c2f62915cb58dc0ee5e43e4096b3e2/apis/v1beta1/gateway_types.go#L76-L78

## API

This proposal has two aspects: configuration for the parent Gateway, and configuration for the child Gateway.

A "parent" Gateway _does not_ reference another Gateway. A parent Gateway MUST explicitly opt into merging.
This is done with a new field `allowedChildren` in the `spec.infrastructure` of a Gateway.
If, and only if, they have done so, the same Gateway is also permitted to leave `listeners` empty (currently, there is a `MinItems=1` restriction).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels logically safe because it gates the removal of the existing restriction on explicitly setting a new optional field, so my understanding is that this wouldn't risk breaking older implementations (a concern previously cited in #1817 (comment) and #1596 (comment)).

@robscott @youngnick thoughts?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely support of this but its worth pointing out that this does not represent a guarantee that the merged set produces MinItems=1 since there may be no children. Thus we cannot completely state that runtime merging is the exact equivalent of a single merged resource.

Given that runtime handling of empty lists by controllers was given as a reason for rejecting

#1596
#1817

we may have to reconsider the logic of that decision given the desire to support merging. E.g.

"if controllers want to support merging as per the spec they must tolerate empty listeners even if merging is not used"

It seems like we are making a similar constraint loosening decision around certs for TLS listeners
#2721

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @louiscryan here. I think it's important to note this tradeoff in the GEP. Even implementations that didn't support Gateway merging would still need to be aware of this potential configuration and ensure that it didn't break them. If we move forward with this approach, we'll need to take extra efforts to communicate the disruptive nature of this change.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I may toss in an opinion here.. I've been watching this proposal because I like the use case of service-based teams and trusting them to manage any needed domains. It would be ideal if these teams didn't have to disrupt core infrastructure resources (e.g. a top-level 80/443 gateway) before deploying their domain.

Could this be more specific about the way allowedChildren should be used? Could it specify an intent to support regex and/or broader matching, e.g. all gateways in a namespace.?

Copy link
Contributor

@mikemorris mikemorris Mar 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cornfeedhobo I'd expect allowedChildren to look very similar to AllowedRoutes, and specifically the namespaces field type RouteNamespaces, which yes would support matching all gateways in one or more namespaces (Same namespace, All namespaces, or by using a label selector match).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing that just occurred to me when rereading this is that changing minItems to 0 for Listeners means that we are moving Listeners from required to optional, which is a breaking API change (because fields that you used to be able to assume would always have at least one entry now do not). Strictly, that would mean we'd have to do an API revision to v2, which does not seem likely.

Realistically, it may not be too much of a problem for Go implementations, if they're doing a range across spec.listeners, since that will execute the code 0 times if the list is empty. But it is a substantial UX change to be able to add Gateways with no Listeners. It's very likely that code will break as a result.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this could be solved conditionally for example we allow minItems == 0 if spec.infrastructure.allowedChildren is not nil which implies the Gateway will be a parent. @robscott is that something that is possible with CEL?

But it is a substantial UX change to be able to add Gateways with no Listeners. It's very likely that code will break as a result.

This doesn't break existing users or implementations. I'd say this is more analogous to supporting a new feature in a newer version of the API.

When an implementation adds 1.2 support they can reject Gateways with no listeners if they do not support gateway merging etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CEL was the intent here and how we implement "If, and only if, they have done so..."

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen on enforcing the parent/child relationship for this. It might not be a bad idea to consider a policy or other CRD to record the members of a group that can merge with each other. Then you are free to design a solution without the restrictions of breaking API changes.


A "child" Gateway references a parent Gateway.
This is done with a new field `attachTo` in the `spec.infrastructure` stanza of a Gateway.
The `attachTo` field is a new type `GatewayObjectReference`.
Although the use of `GatewayObjectReference` allows users to attach to any `kind`, this GEP only defines the behavior of attaching a Gateway to another Gateway.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

A "sibling" is a Gateway that shares a parent with another child Gateway.

Status requirements are specified [below](#status-fields).

See [GEP-1867](https://github.com/kubernetes-sigs/gateway-api/pull/1868) for more use cases of `infrastructure`.


#### Go

```go
type GatewayInfrastructure struct {
// ...

// AllowedChildren allows child objects to attach to this Gateway.
// A common scenario is to allow other objects to add listeners to this Gateway.
AllowedChildren *AllowedChildren `json:"allowedChildren,omitempty"`

// AttachTo allows the Gateway to associate itself with another resource.
// A common scenario is to reference another Gateway which marks
// this Gateway a child of another.
AttachTo GatewayObjectReference `json:"attachTo"`
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
}

// AllowedChildren defines which objects may be attached as children
type AllowedChildren struct {
// Namespaces indicates namespaces from which children may be attached to this
// Gateway. This is restricted to the namespace of this Gateway by default.
//
// Support: Core
//
// +optional
// +kubebuilder:default={from: Same}
Namespaces *ChildrenNamespaces `json:"namespaces,omitempty"`
}

// ChildrenNamespaces indicate which namespaces Children should be selected from.
type ChildrenNamespaces struct {
// From indicates where Children will be selected for this Gateway. Possible
// values are:
//
// * All: Children in all namespaces may be used by this Gateway.
// * Selector: Children in namespaces selected by the selector may be used by
// this Gateway.
// * Same: Only Children in the same namespace may be used by this Gateway.
//
// Support: Core
//
// +optional
// +kubebuilder:default=Same
From *FromNamespaces `json:"from,omitempty"`

// Selector must be specified when From is set to "Selector". In that case,
// only Children in Namespaces matching this Selector will be selected by this
// Gateway. This field is ignored for other values of "From".
//
// Support: Core
//
// +optional
Selector *metav1.LabelSelector `json:"selector,omitempty"`
}

// GatewayObjectReference identifies an API object including its namespace,
// defaulting to Gateway.
type GatewayObjectReference struct {
// Group is the group of the referent. For example, "gateway.networking.k8s.io".
// When unspecified or empty string, core API group is inferred.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
//
// +optional
// +kubebuilder:default=""
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
Group *Group `json:"group"`

// Kind is kind of the referent. For example "Gateway".
//
// +optional
// +kubebuilder:default=Gateway
Kind *Kind `json:"kind"`

// Name is the name of the referent.
Name ObjectName `json:"name"`

// Namespace is the namespace of the referenced object. When unspecified, the local
// namespace is inferred.
//
// Support: Core
//
// +optional
Namespace *Namespace `json:"namespace,omitempty"`
}
```

#### YAML

Below shows an example of an end to end configuration with Gateway merging.
Here we define a parent resource, which allows children from the same namespace.
A single listener is defined in the parent.

The child Gateway attaches to this Gateway and specifies an additional listener.

```yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: parent-gateway
spec:
gatewayClassName: example
infrastructure:
allowedChildren:
namespaces:
from: Same
listeners:
- name: common-monitoring
port: 8080
protocol: HTTP
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: child-gateway
spec:
gatewayClassName: example
infrastructure:
attachTo:
name: parent-gateway
kind: Gateway
group: gateway.networking.k8s.io
listeners:
- name: domain-a
hostname: a.example.com
protocol: HTTP
port: 80
```

Logically, this is equivalent to a single Gateway as below:

```yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: gateway
spec:
gatewayClassName: example
listeners:
- name: common-monitoring
port: 8080
protocol: HTTP
- name: domain-a
hostname: a.example.com
protocol: HTTP
port: 80
Comment on lines +190 to +193
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside of the generated listeners use case, it seems like a primary use case here would be to delegate domains to different namespaces. Unfortunately this doesn't provide any kind of controls for that so it seems like the first namespace to claim a domain would win. I'm concerned that this would become rather chaotic and confusing in practice. A key idea of Gateway listeners was that they allowed you to delegate domains to routes in different namespaces, this seems to make that distinction less clear.

Are there any controls/guardrails we can offer here? Is cross-namespace delegation a key goal? If not, could/should we start without it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, given the following things, which seem on the face of it to be reasonable asks:

  • Gateway merging merges the Listeners of each Gateway into one flat list, essentially
  • Gateway merging can occur across namespaces

There's no way around the fact that conflicts are not predictable by looking at any single namespace plus the parent Gateway. There's no way to know if someone in a sibling Gateway in another namespace is already using the same port/protocol/hostname/TLS config you are, which will put those Listeners into conflict.

By the current rules, Listeners that are conflicting are both rejected, because they must necessarily be created at the same time (since they are in the same object). Assuming that we use our usual conflict-resolution rules, then, as @robscott says above, the first Gateway to claim the any distinct Listener combination will be the winner forever.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this doesn't provide any kind of controls for that so it seems like the first namespace to claim a domain would win. A key idea of Gateway listeners was that they allowed you to delegate domains to routes in different namespaces, this seems to make that distinction less clear.

Not really - it's really more about splitting up the listener list across resources.

A key idea of Gateway listeners was that they allowed you to delegate domains to routes in different namespaces, this seems to make that distinction less clear.

This doesn't change that in my opinion.

Are there any controls/guardrails we can offer here? Is cross-namespace delegation a key goal? If not, could/should we start without it?

@howardjohn @mikemorris you have comments here regarding the cross namespace need?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no way to know if someone in a sibling Gateway in another namespace is already using the same port/protocol/hostname/TLS config you are, which will put those Listeners into conflict.

This isn't really any different than what we have now. eg. if Gateway's aren't visible to Application Developers then they wouldn't know. I agree with your point it becomes more difficult when the resource is split up.

By the current rules, Listeners that are conflicting are both rejected, because they must necessarily be created at the same time (since they are in the same object). Assuming that we use our usual conflict-resolution rules, then, as @robscott says above, the first Gateway to claim the any distinct Listener combination will be the winner forever.

The GEP tweaks the rules such that child Gateways can be rejected and it doesn't affect siblings. So like you said conflict resolution rules apply.

As @robscott says above, the first Gateway to claim the any distinct Listener combination will be the winner forever.

Yup.

Copy link
Contributor

@mikemorris mikemorris Jul 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is cross-namespace delegation a key goal? If not, could/should we start without it?

I think there's two aspects to this:

  • Should a child Gateway be able to attach to a parent Gateway in a different namespace?
  • When a Gateway with attached routes attaches to a parent Gateway, what permissions are required on the parent for merging listeners? Does allowedRoutes config on the parent Gateway apply here, such as if the parent specified allowedRoutes with a selector for only a different namespace? Should we specify allowedRoutes as scoped to only directly attached routes?

https://docs.google.com/document/d/1-0mgRRAY784OgGQ1_LCOshpLLbeAtIr4eXd0YVYK4RY/edit has some context on cross-namespace delegation being an explicit goal for the prior route delegation proposal, but I'm not sure if there's a specific need for it with Gateway merging.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for at least some of the use cases, cross-namespace attachment seems important - which implies that we should have a two-way handshake for safety. Which is difficult with the current object. Sigh. Another reason that @robscott and I have been talking about a new object type instead.

```

### Semantics

#### Route Attaching

Routes MUST be able to specify a child Gateway as a `parentRef` and make use of the fields in `ParentReference` to help target a specific listener.
If no listener is targeted (`sectionName`/`port` are unset) then the Route references all the listeners on the child Gateway. It MUST NOT attach
to a listener on a parent or sibling Gateway.
Comment on lines +201 to +202
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would you enforce this? In the event of conflicts, which one wins?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the question - what conflict are you referring to?


```yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
name: httproute-example
spec:
parentRefs:
- name: child-gateway
sectionName: metrics
```

Routes can only bind to listeners *directly* defined in the `Gateway` referenced.
For instance, the following route referencing a listener defined in the parent `Gateway`, but attaching to the child `Gateway` is not valid.
This will be reported in [status](#status-fields).

```yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: HTTPRoute
metadata:
name: httproute-example
spec:
parentRefs:
- name: child-gateway
sectionName: section-in-parent
```

#### Merging Spec Fields

##### Validation

A parent and child MUST have the same `gatewayClassName`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required or can the child simply omit. Omission is likely better if the root gateway is infrastructure that is being managed centrally. The children don't actually need to know or care about the class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would great to not require it.

I'm not familiar if it's possible with CEL to make a field optional if some other property is set (eg. infrastructure.attachTo`

@robscott do you know - I'd consider you the CEL expert.

Copy link
Contributor Author

@dprotaso dprotaso Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A thing to note is that getting the gateway would show an empty class

// +kubebuilder:printcolumn:name="Class",type=string,JSONPath=`.spec.gatewayClassName`

Unless we made a change so that it appears in the status

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do it, you just need to attach the rule to some common root (here that would be spec), rather than on gatewayClassName

Copy link
Contributor

@mikemorris mikemorris Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does make sense that the class needs to be managed centrally (by the parent Gateway administrator), butspec.gatewayClassName is a required field on Gateway and while the field doesn't appear to actually be immutable, it's recommended that the specified GatewayClass be used as a template when creating a Gateway, so while it's a bit of extra boilerplate I don't expect it's something that should ever be changing dynamically.

What we may want to add is a clarification that about the GatewayClass "template snapshotting" recommendation - this should likely be tied to the parent Gateway rather than attempting to reconcile potentially drifting GatewayClass configuration snapshots from multiple child Gateways created at different points in time.

Copy link

@louiscryan louiscryan Feb 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I get the templating recommendation in principle I think it presents a toil problem for users when you attempt to apply it to the specification of Gateway.gatewayClassName.

Since the compositional model for routes & policies is back-reference the toil fanout of forcing Gateway immutability is high on users. In this situation any class transition (infrastructure, version etc.) would force a reference update on every inbound policy since the user would have to create a new Gateway resource.

Even immutability is somewhat of an illusion since the recommendation can't prevent the deletion & recreation of the resource with the same name and a different value. The user is simply left with the side-effects of their workaround.

Copy link
Contributor

@mikemorris mikemorris Feb 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, I would be in favor of allowing an empty gatewayClassName contingent on attachTo being configured if that's permissible - I just think it's a nicety rather than a necessity because changing a gatewayClassName will often be a destructive action and as such not a significantly different, safer, or less downtime operation than deleting and recreating all child and parent resources.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, gatewayClassName is a required field, and changing it to optional is a breaking API change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can remove gatewayClassName because if we try to attach to a non-existent parent then who would be responsible to update the status of the Gateway saying invalid references etc?

I'm going to resolve this thread end of week unless someone has an idea on solving the situation I just mentioned.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can remove gatewayClassName because if we try to attach to a non-existent parent then who would be responsible to update the status of the Gateway saying invalid references etc?

That makes a lot of sense to me, hadn't considered that situation.

This will be detected by the implementation and reported in [status](#status-fields).

A child resource MUST not set any `spec.infrastructure` fields beyond `attachTo`, and cannot set `spec.address`.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
This can be validated in the CRD schema.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

A parent resource MUST not set `spec.infrastructure.attachTo`.
That is, we do not allow multiple tiers of Gateways chaining to each other; there is only a single parent with children.
This can be validated in the CRD schema.

A child resource cannot `attachTo` any Gateway resource that doesn't allow attachment (eg. it does not specify `spec.infrastructure.allowedChildren` for `Gateway`s).
This will be detected by the implementation and reported in [status](#status-fields).

##### Listeners

Implementations MUST treat the "parent" Gateway as having the concatenated list of all listeners from itself and "child" Gateways.

Validation of this list of listeners MUST behave the same as if the list were part of a single "parent" Gateway.

eg.
```yaml
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: parent-gateway
spec:
infrastructure:
allowedChildren:
namespaces:
from: Same
gatewayClassName: example
listeners:
- name: HTTP
port: 80
protocol: HTTP
---
apiVersion: gateway.networking.k8s.io/v1beta1
kind: Gateway
metadata:
name: child-gateway
spec:
gatewayClassName: example
infrastructure:
attachTo:
name: parent-gateway
kind: Gateway
group: gateway.networking.k8s.io
listeners:
- name: metrics
port: 8080
protocol: HTTP
```

With this configuration, the realized "parent" Gateway should listen on port `80` and `8080`.

###### Listener Precedence

Gateway Listeners should be merged using the following precedence:
- "parent" Gateway
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
- "child" Gateway ordered by creation time (oldest first)
- "child" Gateway ordered alphabetical by “{namespace}/{name}”.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

If there are conflicts between these, this should be reported as `Conflicted=True` in the listener as usual.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

### Status Fields

#### Addresses

The list of `Addresses` that appear in the status of the "child" Gateway MUST be the same as the "parent" Gateway.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

#### Gateway Conditions
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

Gateway conditions currently supports the following condition types: `Accepted` and `Programmed`

For parent gateways, `Accepted` should be set based on the entire set of merged listeners.
For instance, if a child listener is invalid, `ListenersNotValid` would be reported.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
`Programmed` is not expected, generally, to depend on the children resources, but if an implementation does depend on these
they should consider child resources when reporting this status.

For child gateways, `Accepted` and `Programmed` should not consider the overall merged Gateway status, but only the child's own listeners.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
The exception to this is if the parent Gateway is not valid in any regard.

For example, if I have a `parent`, `child-1`, and `child-2`:
* If `parent` is entirely invalid (for example, an invalid `address`), all three Gateways will reported `Accepted=False`.
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
* If `child-1` has an invalid listener, `parent` and `child-1` will report `ListenersNotValid`, while `child-2` will not.
* If `child-1` references a parent that doesn't exist then `child-1` will report `Accepted=False`
* If `child-1` references a parent that doesn't allow merging then `child-1` will report `Accepted=False`
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
* If `child-1` references another child (eg. `child-2`) then `child-1` will report `Accepted=False`
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
* If `child-1` references itself then `child-1` will report `Accepted=False`
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
* If `child-1` and `parent` have different gatewayClassNames then `child-1` will report `Accepted=False`
dprotaso marked this conversation as resolved.
Show resolved Hide resolved
* If `child-1` references a parent whose allowed child namespaces do not include `child-1`'s namespace, then `child-1` will report `Accepted=False`
Copy link
Contributor

@youngnick youngnick Jun 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One other scenario I think we need to work through here:

parent, child-1, child-2, and child-3.

  • All three children have multiple listeners.
  • child-2 and child-3 have one listener that conflicts.
  • Let's say child-2 was created first.

What happens?

How do the owners of each child know what's happened? (Preferably without looking at the parent's status)

What actions do the owners of child-2 and child-3 need to take to fix this? Can they know that which other sibling Gateway has caused the problem? If child-2 wins precedence because of age, how do they know that something tried to claim their config and failed?

Note that in this case, if child-2 ever deletes and recreates their Gateway, they will now be in the same position as the owner of child-3.

How does the owner of the parent know if any children are bad, and is it possible for them to know which ones?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens?

child-3 would be rejected and child-2 wouldn't change. I'll add a condition about said conflict - I thought this was covered elsewhere.

How do the owners of each child know what's happened? (Preferably without looking at the parent's status)

The status on the child could state an appropriate error message.

What actions do the owners of child-2 and child-3 need to take to fix this?

Gateway owners (cluster operator) would have to address this by tweaking listeners.

Can they know that which other sibling Gateway has caused the problem?

I think it's worth the error message saying there's a conflict and providing the more detailed message about siblings on the parent's status message.

If child-2 wins precedence because of age, how do they know that something tried to claim their config and failed?

Do you have a use case why child-2 would want to know this information?

Note that in this case, if child-2 ever deletes and recreates their Gateway, they will now be in the same position as the owner of child-3.

Yup

How does the owner of the parent know if any children are bad, and is it possible for them to know which ones?

It's mentioned that ListenersNotValid will appear on the parent's status. A more detailed message can be presented.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the scenario here 0de79b6


When reporting status of a child, an implementation SHOULD be cautious about what information from the parent or siblings are reported
to avoid accidentally leaking sensitive information that the child would not otherwise have access to.

#### Listener Conditions
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

Listener conditions should only be set for listeners directly defined in a given Gateway.
Parent gateways MUST NOT have children's resources in their listener conditions list.
Children gateways MUST NOT have parent's or sibling's resources in their listener conditions list.
An implementation MAY reject Listeners with ListenerConditionAccepted=False and Reason "TooManyListeners"

#### Policy attachment

Policies attached to a parent Gateway apply to both the parent and all children listeners.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to consider or rule out an alternative that is defined in a policy that could be attached to the parent Gateway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate what you mean? I don't understand.


Policies that attach to a child Gateway apply to all listeners defined in that Gateway, but do not impact
parent or sibling listeners.
If the implementation cannot apply the policy to only specific listeners, it should reject the policy.

## Future Goals

### Requirement Level

We want to keep this API very simple so that the merging requirement level could increase from `MAY` to `MUST`

## Alternatives
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

#### New Resource
A `GatewayListener` resource could be a simpler solution as we would not have to set required fields (ie. gatewayClassName)

```
apiVersion: gateway.networking.k8s.io/v1beta1
kind: GatewayListener
metadata:
name: listener
spec:
gateway: parent-gateway
listeners:
- name: metrics
port: 8080
protocol: HTTP
status: ...
```

#### Use of the `gateway.networking.k8s.io/parent-gateway` label

Use of a label (ie. `gateway.networking.k8s.io/parent-gateway: name`) could be used to select child gateways vs using `spec.infrastructure.attachTo`
dprotaso marked this conversation as resolved.
Show resolved Hide resolved

## References

Mentioned in Prior GEPs:
- https://github.com/kubernetes-sigs/gateway-api/pull/1757

Prior Discussions:
- https://github.com/kubernetes-sigs/gateway-api/discussions/1248
- https://github.com/kubernetes-sigs/gateway-api/discussions/1246
8 changes: 8 additions & 0 deletions geps/gep-1713/metadata.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
apiVersion: internal.gateway.networking.k8s.io/v1alpha1
kind: GEPDetails
number: 1713
name: Gateway Merging
status: Provisional
authors:
- dprotaso
- howardjohn