Skip to content

Commit d1cad24

Browse files
hbagdihbagdi
committed
gep-718: rework RoueForwardTo
1 parent a9d45b5 commit d1cad24

File tree

1 file changed

+339
-0
lines changed

1 file changed

+339
-0
lines changed

‎site-src/geps/gep-718.md

+339
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,339 @@
1+
# GEP-718: Rework forwardTo segment in routes
2+
3+
Issue URL: https://github.com/kubernetes-sigs/gateway-api/issues/718
4+
5+
Status: implementable
6+
7+
## Motivation
8+
9+
### Complications due to the `serviceName` shortcut
10+
11+
Within `RouteForwardTo` (and `HTTPRouteForwardTo` by extension) there exists
12+
two mechanisms to specify the upstream network endpoint where the traffic should
13+
be forwarded to:
14+
- `ServiceName`: the name of the Kubernetes Service
15+
- `BackendRef`: a LocalObjectReference to a resource, this is an extension point in the API
16+
17+
While working on [#685](https://github.com/kubernetes-sigs/gateway-api/pull/685),
18+
it came to light that:
19+
1. `BackendRef` itself could be used to point to a Kubernetes Service
20+
2. With [GEP-709](https://github.com/kubernetes-sigs/gateway-api/pull/711),
21+
there will be a need to introduce a new `namespace` field which will enable use-cases to forward
22+
traffic to a services and backends located in different namespaces
23+
24+
While (1) can be fixed with more validation and documentation, it only solves for
25+
the specific case. (2) requires adding two fields: `serviceNamespace`
26+
and `BackendRef.Namespace` - something we would like to avoid since the `serviceName`
27+
field was introduced only as a shortcut and adding more service-specific fields
28+
defeats the original purpose.
29+
30+
These problems are in addition to the original problem that
31+
[#685](https://github.com/kubernetes-sigs/gateway-api/pull/685) attempts to solve:
32+
clarifying port requirements when a port is required or not.
33+
We have been updating documentation to tackle such corner-cases but that
34+
continues to result in more and more elaborations. Some excerpts from our
35+
existing documentation:
36+
```
37+
// ServiceName refers to the name of the Service to mirror matched requests
38+
// to. When specified, this takes the place of BackendRef. If both
39+
// BackendRef and ServiceName are specified, ServiceName will be given
40+
// precedence.
41+
42+
// BackendRef is a local object reference to mirror matched requests to. If
43+
// both BackendRef and ServiceName are specified, ServiceName will be given
44+
// precedence.
45+
46+
// Weight specifies the proportion of HTTP requests forwarded to the backend
47+
// referenced by the ServiceName or BackendRef field. This is computed as
48+
49+
// Port specifies the destination port number to use for the
50+
// backend referenced by the ServiceName or BackendRef field.
51+
// If unspecified, the destination port in the request is used
52+
// when forwarding to a backendRef or serviceName.
53+
```
54+
55+
### Simplifying `RouteForwardTo`
56+
57+
RouteForwardTo is not the best of the names. Using a noun instead of a verb
58+
would help with in-person communication and documentation.
59+
Since we are already considering dropping the special case of `service` as a
60+
backend, we have an opportunity to further simplify `RouteForwardTo` for better
61+
UX.
62+
63+
## Proposal
64+
65+
- Remove the `ServiceName` field from `RouteForwardTo`
66+
- Introduce `Service` as a default for `BackendRef.Kind`
67+
- Rename `RouteForwardTo.BackendRef` to `RouteForwardTo.TargetRef`
68+
- Rename `RouteForwardTo` to `Backend`
69+
- Rename `Route.Spec.Rules[].ForwardTo[]` to `Route.Spec.Rules[].Backends[]` in
70+
all routes
71+
- Apply the same to HTTP types and filters
72+
73+
## API
74+
75+
The updated `Backend` (formerly `RouteForwardTo`) struct will be:
76+
(HTTP version has been omitted for brevity)
77+
78+
```go
79+
// Backend defines how and where a request should be forwarded from the Gateway.
80+
type Backend struct {
81+
// TargetRef is a reference to the resource to forward matched requests to.
82+
//
83+
// If the referent cannot be found, the rule is not included in the route.
84+
// The controller should raise the "ResolvedRefs" condition on the Gateway
85+
// with the "DegradedRoutes" reason. The gateway status for this route should
86+
// be updated with a condition that describes the error more specifically.
87+
//
88+
// The protocol to use is defined using AppProtocol field (introduced in
89+
// Kubernetes 1.18) in the Service resource. In the absence of the
90+
// AppProtocol field a `gateway.networking.k8s.io/app-protocol` annotation on the
91+
// BackendPolicy resource may be used to define the protocol. If the
92+
// AppProtocol field is available, this annotation should not be used. The
93+
// AppProtocol field, when populated, takes precedence over the annotation
94+
// in the BackendPolicy resource. For custom backends, it is encouraged to
95+
// add a semantically-equivalent field in the Custom Resource Definition.
96+
//
97+
// Support: Core (Kubernetes Service)
98+
// Support: Custom (any other resource)
99+
//
100+
// +optional
101+
// +kubebuilder:default={kind: "service"}
102+
TargetRef *LocalObjectReference `json:"targetRef,omitempty"`
103+
104+
// Port specifies the destination port number to use for the
105+
// backend referenced by the TargetRef field.
106+
// This field is required when the target is a Kubernetes Service.
107+
//
108+
// Support: Core
109+
//
110+
// +optional
111+
Port *PortNumber `json:"port,omitempty"`
112+
113+
// Weight specifies the proportion of HTTP requests forwarded to the backend
114+
// referenced by the TargetRef field. This is computed as
115+
// weight/(sum of all weights in this Backends list). For non-zero values,
116+
// there may be some epsilon from the exact proportion defined here
117+
// depending on the precision an implementation supports. Weight is not a
118+
// percentage and the sum of weights does not need to equal 100.
119+
//
120+
// If only one backend is specified and it has a weight greater than 0, 100%
121+
// of the traffic is forwarded to that backend. If weight is set to 0, no
122+
// traffic should be forwarded for this entry. If unspecified, weight
123+
// defaults to 1.
124+
//
125+
// Support: Extended
126+
//
127+
// +optional
128+
// +kubebuilder:default=1
129+
// +kubebuilder:validation:Minimum=0
130+
// +kubebuilder:validation:Maximum=1000000
131+
Weight *int32 `json:"weight,omitempty"`
132+
}
133+
134+
```
135+
136+
A summarized diff for the changes being proposed:
137+
138+
```patch
139+
diff --git a/apis/v1alpha2/shared_types.go b/apis/v1alpha2/shared_types.go
140+
index 1215300..c5afddb 100644
141+
--- a/apis/v1alpha2/shared_types.go
142+
+++ b/apis/v1alpha2/shared_types.go
143+
@@ -94,12 +94,9 @@ type GatewayReference struct {
144+
Namespace string `json:"namespace"`
145+
}
146+
147+
-// RouteForwardTo defines how a Route should forward a request.
148+
-type RouteForwardTo struct {
149+
- // ServiceName refers to the name of the Service to forward matched requests
150+
- // to. When specified, this takes the place of BackendRef. If both
151+
- // BackendRef and ServiceName are specified, ServiceName will be given
152+
- // precedence.
153+
+// Backend defines how and where a request should be forwarded from the Gateway.
154+
+type Backend struct {
155+
+ // TargetRef is a reference to the resource to forward matched requests to.
156+
//
157+
// If the referent cannot be found, the rule is not included in the route.
158+
// The controller should raise the "ResolvedRefs" condition on the Gateway
159+
@@ -115,30 +112,16 @@ type RouteForwardTo struct {
160+
// in the BackendPolicy resource. For custom backends, it is encouraged to
161+
// add a semantically-equivalent field in the Custom Resource Definition.
162+
//
163+
- // Support: Core
164+
- //
165+
- // +optional
166+
- // +kubebuilder:validation:MaxLength=253
167+
- ServiceName *string `json:"serviceName,omitempty"`
168+
-
169+
- // BackendRef is a reference to a backend to forward matched requests to. If
170+
- // both BackendRef and ServiceName are specified, ServiceName will be given
171+
- // precedence.
172+
- //
173+
- // If the referent cannot be found, the rule is not included in the route.
174+
- // The controller should raise the "ResolvedRefs" condition on the Gateway
175+
- // with the "DegradedRoutes" reason. The gateway status for this route should
176+
- // be updated with a condition that describes the error more specifically.
177+
- //
178+
- // Support: Custom
179+
+ // Support: Core (Kubernetes Service)
180+
+ // Support: Custom (any other resource)
181+
//
182+
// +optional
183+
- BackendRef *LocalObjectReference `json:"backendRef,omitempty"`
184+
+ // +kubebuilder:default={kind: "service"}
185+
+ TargetRef *LocalObjectReference `json:"targetRef,omitempty"`
186+
187+
// Port specifies the destination port number to use for the
188+
- // backend referenced by the ServiceName or BackendRef field.
189+
- // If unspecified, the destination port in the request is used
190+
- // when forwarding to a backendRef or serviceName.
191+
+ // backend referenced by the TargetRef field.
192+
+ // This field is required when the target is a Kubernetes Service.
193+
//
194+
// Support: Core
195+
//
196+
@@ -146,8 +129,8 @@ type RouteForwardTo struct {
197+
Port *PortNumber `json:"port,omitempty"`
198+
199+
// Weight specifies the proportion of HTTP requests forwarded to the backend
200+
- // referenced by the ServiceName or BackendRef field. This is computed as
201+
- // weight/(sum of all weights in this ForwardTo list). For non-zero values,
202+
+ // referenced by the TargetRef field. This is computed as
203+
+ // weight/(sum of all weights in this Backends list). For non-zero values,
204+
// there may be some epsilon from the exact proportion defined here
205+
// depending on the precision an implementation supports. Weight is not a
206+
// percentage and the sum of weights does not need to equal 100.
207+
```
208+
209+
## Examples
210+
211+
212+
For Kubernetes Services, an updated `forwardTo` section will read as follows:
213+
214+
```yaml
215+
...
216+
backends:
217+
- targetRef:
218+
name: foo-service-v1
219+
port: 80
220+
weight: 80
221+
- targetRef:
222+
name: foo-service-canary
223+
port: 80
224+
weight: 20
225+
...
226+
```
227+
228+
Here, the `kind` field is omitted as it will be injected as a default.
229+
230+
For custom backends, the API will look like the following:
231+
232+
```yaml
233+
...
234+
backends:
235+
- targetRef:
236+
name: foo-v1
237+
kind: server
238+
group: networking.acme.io
239+
port: 80
240+
weight: 80
241+
- targetRef:
242+
name: foo-v1-canary
243+
kind: server
244+
group: networking.acme.io
245+
port: 80
246+
weight: 20
247+
...
248+
```
249+
250+
For completeness, here is an example of HTTPRoute:
251+
252+
```yaml
253+
kind: HTTPRoute
254+
apiVersion: gateway.networking.k8s.io/v1alpha2
255+
metadata:
256+
name: bar-route
257+
labels:
258+
gateway: prod-web-gw
259+
spec:
260+
hostnames:
261+
- "bar.example.com"
262+
rules:
263+
- matches:
264+
- headers:
265+
type: Exact
266+
values:
267+
env: canary
268+
backends:
269+
- targetRef:
270+
name: foo-service-v1
271+
port: 80
272+
weight: 80
273+
- targetRef:
274+
name: foo-service-canary
275+
port: 80
276+
weight: 20
277+
```
278+
279+
## Alternatives considered
280+
281+
### Rename serviceName to backendName
282+
283+
As suggested in [this comment](https://github.com/kubernetes-sigs/gateway-api/pull/685#discussion_r667285837),
284+
we could buy the best of both the worlds by introducing `backendName`:
285+
286+
```yaml
287+
forwardTo:
288+
- backendName: foo
289+
port: 80
290+
kind: <defaults to Service>
291+
```
292+
293+
While this saves one line of YAML (`backendRef:`), it could be potentially
294+
violating the `Naming of the reference field`
295+
[API convention][api-convetions].
296+
Most of our object references are of the form `XRef`.
297+
298+
### Flatten `TargetRef` into `Backend` itself
299+
300+
A further simplification is possible in this case if the `TargetRef` struct
301+
is embedded directly inside `Backend`:
302+
303+
```yaml
304+
...
305+
backends:
306+
- name: foo-service-v1
307+
port: 80
308+
weight: 80
309+
- name: foo-service-canary
310+
port: 80
311+
weight: 20
312+
...
313+
```
314+
315+
This result in saving of one line of YAML per backend.
316+
This has been avoided for the following reasons:
317+
- Separate the fields that are present for object referencing purposes from
318+
fields that are specific to Gateway API and influence routing behavior.
319+
- [API conventions][api-conventions] are unclear about folding such fields
320+
directly within a struct that contains other information. Upstream APIs
321+
seem to separate out references from other fields via a `fooRef` struct.
322+
- If we were to do this, this would be a new pattern within this API as there
323+
is no other place where such a flattening is performed. Future maintainers
324+
could reference this pattern and further worsen the API experience.
325+
326+
## Out of scope
327+
328+
N/A
329+
330+
## References
331+
332+
Existing documentation:
333+
- [RouteForwardTo](https://github.com/kubernetes-sigs/gateway-api/blob/a9d45b51c396fbed022204af0185b00a4ac7e282/apis/v1alpha2/shared_types.go#L97-L167)
334+
- [HTTPRouteForwardTo](https://github.com/kubernetes-sigs/gateway-api/blob/a9d45b51c396fbed022204af0185b00a4ac7e282/apis/v1alpha2/httproute_types.go#L731-L813)
335+
- [#685](https://github.com/kubernetes-sigs/gateway-api/pull/685)
336+
- [Comment thread that spawned this GEP](https://github.com/kubernetes-sigs/gateway-api/pull/685#discussion_r640204333)
337+
- [#578](https://github.com/kubernetes-sigs/gateway-api/issues/578)
338+
339+
[api-conventions]: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#naming-of-the-reference-field

0 commit comments

Comments
 (0)