|
| 1 | +# GEP-718: Rework forwardTo segment in routes |
| 2 | + |
| 3 | +Issue: [#718](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 | +- Pull fields from `RouteForwardTo.BackendRef` into `RouteForwardTo` itself |
| 68 | +- Rename `RouteForwardTo` to `BackendRef` |
| 69 | +- Rename `Route.Spec.Rules[].ForwardTo[]` to `Route.Spec.Rules[].BackendRefs[]` in |
| 70 | + all routes |
| 71 | +- Apply the same to HTTP types and filters |
| 72 | + |
| 73 | +## API |
| 74 | + |
| 75 | +The updated `BackendRef` (formerly `RouteForwardTo`) struct will be: |
| 76 | +(HTTP version has been omitted for brevity) |
| 77 | + |
| 78 | +```go |
| 79 | +// BackendRef defines how and where a request should be forwarded from the Gateway. |
| 80 | +type BackendRef struct { |
| 81 | + // Group is the group of the backend resource. |
| 82 | + // |
| 83 | + // +kubebuilder:validation:MinLength=1 |
| 84 | + // +kubebuilder:validation:MaxLength=253 |
| 85 | + Group string `json:"group"` |
| 86 | + |
| 87 | + // Kind is kind of the backend resource. |
| 88 | + // |
| 89 | + // Support: Core (Kubernetes Service) |
| 90 | + // Support: Custom (any other resource) |
| 91 | + // |
| 92 | + // +optional |
| 93 | + // +kubebuilder:validation:MinLength=1 |
| 94 | + // +kubebuilder:validation:MaxLength=253 |
| 95 | + // +kubebuilder:default="service" |
| 96 | + Kind *string `json:"kind"` |
| 97 | + |
| 98 | + // Name is the name of the backend resource to forward matched requests to. |
| 99 | + // |
| 100 | + // If the referent cannot be found, the rule is not included in the route. |
| 101 | + // The controller should raise the "ResolvedRefs" condition on the Gateway |
| 102 | + // with the "DegradedRoutes" reason. The gateway status for this route should |
| 103 | + // be updated with a condition that describes the error more specifically. |
| 104 | + // |
| 105 | + // +kubebuilder:validation:MinLength=1 |
| 106 | + // +kubebuilder:validation:MaxLength=253 |
| 107 | + Name string `json:"name"` |
| 108 | + |
| 109 | + // Port specifies the destination port number to use for the |
| 110 | + // backend resource. |
| 111 | + // This field is required when the backend is a Kubernetes Service. |
| 112 | + // |
| 113 | + // Support: Core |
| 114 | + // |
| 115 | + // +optional |
| 116 | + Port *PortNumber `json:"port,omitempty"` |
| 117 | + |
| 118 | + // Weight specifies the proportion of HTTP requests forwarded to the backend |
| 119 | + // This is computed as weight/(sum of all weights in this Backends list). |
| 120 | + // For non-zero values, there may be some epsilon from the exact proportion |
| 121 | + // defined here depending on the precision an implementation supports. Weight |
| 122 | + // is not a percentage and the sum of weights does not need to equal 100. |
| 123 | + // |
| 124 | + // If only one backend is specified and it has a weight greater than 0, 100% |
| 125 | + // of the traffic is forwarded to that backend. If weight is set to 0, no |
| 126 | + // traffic should be forwarded for this entry. If unspecified, weight |
| 127 | + // defaults to 1. |
| 128 | + // |
| 129 | + // Support: Extended |
| 130 | + // |
| 131 | + // +optional |
| 132 | + // +kubebuilder:default=1 |
| 133 | + // +kubebuilder:validation:Minimum=0 |
| 134 | + // +kubebuilder:validation:Maximum=1000000 |
| 135 | + Weight *int32 `json:"weight,omitempty"` |
| 136 | +} |
| 137 | +``` |
| 138 | + |
| 139 | +A summarized diff for the changes being proposed: |
| 140 | + |
| 141 | +```patch |
| 142 | +diff --git a/apis/v1alpha1/shared_types.go b/apis/v1alpha1/shared_types.go |
| 143 | +index 458145f..de720cd 100644 |
| 144 | +--- a/apis/v1alpha1/shared_types.go |
| 145 | ++++ b/apis/v1alpha1/shared_types.go |
| 146 | +@@ -94,51 +94,39 @@ type GatewayReference struct { |
| 147 | + Namespace string `json:"namespace"` |
| 148 | + } |
| 149 | + |
| 150 | +-// RouteForwardTo defines how a Route should forward a request. |
| 151 | +-type RouteForwardTo struct { |
| 152 | +- // ServiceName refers to the name of the Service to forward matched requests |
| 153 | +- // to. When specified, this takes the place of BackendRef. If both |
| 154 | +- // BackendRef and ServiceName are specified, ServiceName will be given |
| 155 | +- // precedence. |
| 156 | ++// BackendRef defines how and where a request should be forwarded from the Gateway. |
| 157 | ++type BackendRef struct { |
| 158 | ++ // Name is the name of the backend resource to forward matched requests to. |
| 159 | + // |
| 160 | + // If the referent cannot be found, the rule is not included in the route. |
| 161 | + // The controller should raise the "ResolvedRefs" condition on the Gateway |
| 162 | + // with the "DegradedRoutes" reason. The gateway status for this route should |
| 163 | + // be updated with a condition that describes the error more specifically. |
| 164 | + // |
| 165 | +- // The protocol to use is defined using AppProtocol field (introduced in |
| 166 | +- // Kubernetes 1.18) in the Service resource. In the absence of the |
| 167 | +- // AppProtocol field a `networking.x-k8s.io/app-protocol` annotation on the |
| 168 | +- // BackendPolicy resource may be used to define the protocol. If the |
| 169 | +- // AppProtocol field is available, this annotation should not be used. The |
| 170 | +- // AppProtocol field, when populated, takes precedence over the annotation |
| 171 | +- // in the BackendPolicy resource. For custom backends, it is encouraged to |
| 172 | +- // add a semantically-equivalent field in the Custom Resource Definition. |
| 173 | ++ // +kubebuilder:validation:MinLength=1 |
| 174 | ++ // +kubebuilder:validation:MaxLength=253 |
| 175 | ++ Name string `json:"name"` |
| 176 | ++ |
| 177 | ++ // Kind is kind of the backend resource. |
| 178 | + // |
| 179 | +- // Support: Core |
| 180 | ++ // Support: Core (Kubernetes Service) |
| 181 | ++ // Support: Custom (any other resource) |
| 182 | + // |
| 183 | + // +optional |
| 184 | ++ // +kubebuilder:validation:MinLength=1 |
| 185 | + // +kubebuilder:validation:MaxLength=253 |
| 186 | +- ServiceName *string `json:"serviceName,omitempty"` |
| 187 | ++ // +kubebuilder:default="service" |
| 188 | ++ Kind *string `json:"kind"` |
| 189 | + |
| 190 | +- // BackendRef is a reference to a backend to forward matched requests to. If |
| 191 | +- // both BackendRef and ServiceName are specified, ServiceName will be given |
| 192 | +- // precedence. |
| 193 | +- // |
| 194 | +- // If the referent cannot be found, the rule is not included in the route. |
| 195 | +- // The controller should raise the "ResolvedRefs" condition on the Gateway |
| 196 | +- // with the "DegradedRoutes" reason. The gateway status for this route should |
| 197 | +- // be updated with a condition that describes the error more specifically. |
| 198 | ++ // Group is the group of the backend resource. |
| 199 | + // |
| 200 | +- // Support: Custom |
| 201 | +- // |
| 202 | +- // +optional |
| 203 | +- BackendRef *LocalObjectReference `json:"backendRef,omitempty"` |
| 204 | ++ // +kubebuilder:validation:MinLength=1 |
| 205 | ++ // +kubebuilder:validation:MaxLength=253 |
| 206 | ++ Group string `json:"group"` |
| 207 | + |
| 208 | + // Port specifies the destination port number to use for the |
| 209 | +- // backend referenced by the ServiceName or BackendRef field. |
| 210 | +- // If unspecified, the destination port in the request is used |
| 211 | +- // when forwarding to a backendRef or serviceName. |
| 212 | ++ // backend resource. |
| 213 | ++ // This field is required when the backend is a Kubernetes Service. |
| 214 | + // |
| 215 | + // Support: Core |
| 216 | + // |
| 217 | +@@ -146,11 +134,10 @@ type RouteForwardTo struct { |
| 218 | + Port *PortNumber `json:"port,omitempty"` |
| 219 | + |
| 220 | + // Weight specifies the proportion of HTTP requests forwarded to the backend |
| 221 | +- // referenced by the ServiceName or BackendRef field. This is computed as |
| 222 | +- // weight/(sum of all weights in this ForwardTo list). For non-zero values, |
| 223 | +- // there may be some epsilon from the exact proportion defined here |
| 224 | +- // depending on the precision an implementation supports. Weight is not a |
| 225 | +- // percentage and the sum of weights does not need to equal 100. |
| 226 | ++ // This is computed as weight/(sum of all weights in this Backends list). |
| 227 | ++ // For non-zero values, there may be some epsilon from the exact proportion |
| 228 | ++ // defined here depending on the precision an implementation supports. Weight |
| 229 | ++ // is not a percentage and the sum of weights does not need to equal 100. |
| 230 | + // |
| 231 | + // If only one backend is specified and it has a weight greater than 0, 100% |
| 232 | + // of the traffic is forwarded to that backend. If weight is set to 0, no |
| 233 | +``` |
| 234 | + |
| 235 | +## Examples |
| 236 | + |
| 237 | + |
| 238 | +For Kubernetes Services, an updated `forwardTo` section will read as follows: |
| 239 | + |
| 240 | +```yaml |
| 241 | +... |
| 242 | +backendRefs: |
| 243 | +- name: foo-service-v1 |
| 244 | + port: 80 |
| 245 | + weight: 80 |
| 246 | +- name: foo-service-canary |
| 247 | + port: 80 |
| 248 | + weight: 20 |
| 249 | +... |
| 250 | +``` |
| 251 | + |
| 252 | +Here, the `kind` field is omitted as it will be injected as a default. |
| 253 | + |
| 254 | +For custom backends, the API will look like the following: |
| 255 | + |
| 256 | +```yaml |
| 257 | +... |
| 258 | +backendRefs: |
| 259 | +- name: foo-v1 |
| 260 | + kind: server |
| 261 | + group: networking.acme.io |
| 262 | + port: 80 |
| 263 | + weight: 80 |
| 264 | +- name: foo-v1-canary |
| 265 | + kind: server |
| 266 | + group: networking.acme.io |
| 267 | + port: 80 |
| 268 | + weight: 20 |
| 269 | +... |
| 270 | +``` |
| 271 | + |
| 272 | +For completeness, here is an example of HTTPRoute: |
| 273 | + |
| 274 | +```yaml |
| 275 | +kind: HTTPRoute |
| 276 | +apiVersion: gateway.networking.k8s.io/v1alpha2 |
| 277 | +metadata: |
| 278 | + name: bar-route |
| 279 | + labels: |
| 280 | + gateway: prod-web-gw |
| 281 | +spec: |
| 282 | + hostnames: |
| 283 | + - "bar.example.com" |
| 284 | + rules: |
| 285 | + - matches: |
| 286 | + - headers: |
| 287 | + type: Exact |
| 288 | + values: |
| 289 | + env: canary |
| 290 | + backendRefs: |
| 291 | + - name: foo-service-v1 |
| 292 | + port: 80 |
| 293 | + weight: 80 |
| 294 | + - name: foo-service-canary |
| 295 | + port: 80 |
| 296 | + weight: 20 |
| 297 | +``` |
| 298 | +
|
| 299 | +## Alternatives considered |
| 300 | +
|
| 301 | +### Rename serviceName to backendName |
| 302 | +
|
| 303 | +As suggested in [this comment](https://github.com/kubernetes-sigs/gateway-api/pull/685#discussion_r667285837), |
| 304 | +we could buy the best of both the worlds by introducing `backendName`: |
| 305 | + |
| 306 | +```yaml |
| 307 | +forwardTo: |
| 308 | +- backendName: foo |
| 309 | + port: 80 |
| 310 | + kind: <defaults to Service> |
| 311 | +``` |
| 312 | + |
| 313 | +While this saves one line of YAML (`backendRef:`), it could be potentially |
| 314 | +violating the `Naming of the reference field` |
| 315 | +[API convention][api-convetions]. |
| 316 | +Most of our object references are of the form `XRef`. |
| 317 | + |
| 318 | +## Concerns resolved |
| 319 | + |
| 320 | +A concern was raised around flattening of object reference fields i.e. including |
| 321 | +port and weight field alongside object reference is by k8s API conventions or not. |
| 322 | +This is not a problem and has been confirmed with API maintainers |
| 323 | +([comment](https://github.com/kubernetes-sigs/gateway-api/pull/719#discussion_r678555744)). |
| 324 | + |
| 325 | +## Out of scope |
| 326 | + |
| 327 | +N/A |
| 328 | + |
| 329 | +## References |
| 330 | + |
| 331 | +Existing documentation: |
| 332 | +- [RouteForwardTo](https://github.com/kubernetes-sigs/gateway-api/blob/a9d45b51c396fbed022204af0185b00a4ac7e282/apis/v1alpha2/shared_types.go#L97-L167) |
| 333 | +- [HTTPRouteForwardTo](https://github.com/kubernetes-sigs/gateway-api/blob/a9d45b51c396fbed022204af0185b00a4ac7e282/apis/v1alpha2/httproute_types.go#L731-L813) |
| 334 | +- [#685](https://github.com/kubernetes-sigs/gateway-api/pull/685) |
| 335 | + - [Comment thread that spawned this GEP](https://github.com/kubernetes-sigs/gateway-api/pull/685#discussion_r640204333) |
| 336 | +- [#578](https://github.com/kubernetes-sigs/gateway-api/issues/578) |
| 337 | + |
| 338 | +[api-conventions]: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#naming-of-the-reference-field |
0 commit comments