Refactors gateway api type to use label selectors

[danehans]

Previously, the Gateway API specified a Route local object reference for route association. Using label selectors provides a more flexible mechanism to associate a Gateway with one or more routes.

/cc @bowei

[howardjohn]

How do you define what kind of route to get if we use this approach?

[danehans]

@howardjohn all routes will be associated to all gateways unless a RouteSelector is specified. A namespace and/or a route may contain one or more labels used for gateway association.

Here's a LabelSelector example where routes of two different kinds (HttpRoute and TcpRoute) are associated to gateway sample and HttpRoute "foo" is associated to gateway foo (no routeSelector is defined):

apiVersion: networking.x.k8s.io/v1alpha1
kind: Gateway
metadata:
  name: sample
  namespace: gateways
  spec:
    routeSelector:
      labelSelector:
        app: samples
  <SNIP>
---
apiVersion: networking.x.k8s.io/v1alpha1
kind: Gateway
metadata:
  name: foo
  namespace: gateways
  spec:
    <SNIP>
---
apiVersion: networking.x.k8s.io/v1alpha1
kind: HttpRoute
metadata:
  name: sample
  namespace: samples
  labels:
    app: samples
spec:
  <SNIP>
---
apiVersion: networking.x.k8s.io/v1alpha1
kind: TcpRoute
metadata:
  name: sample
  namespace: samples
  labels:
    app: samples
spec:
  <SNIP>
---
apiVersion: networking.x.k8s.io/v1alpha1
kind: HttpRoute
metadata:
  name: foo
  namespace: samples
  labels:
    app: foo
    version: bar
spec:
  <SNIP>

[howardjohn]

I am a bit worried about how this would be implemented. routes may select objects other than just HttpRoute and TcpRoute. With this approach, an implementer would need to to query for all objects for the label? I suppose they could limit it to the set of objects that they consider valid routes though

[danehans]

routes may select objects other than just HttpRoute and TcpRoute.

@howardjohn can you help me better understand what you mean here? It's my understanding that routes may be chained together for delegation purposes. In the delegation use case, the labels of the top-level route will be used for gateway association. Take a look at #12 (comment) about adding Routes to RouteSelector to support specific route association.

Thanks for the review and feedback.

[ironcladlou]

My understanding of @howardjohn's concern is that TypedLocalObjectReference provides explicit bounds for the set of object types to query, which seems inherently more efficient. A query using an untyped selector is intrinsically unbounded along the object kind dimension.

Does that make sense?

[ironcladlou]

The efficiency aspect may become more important if selection across all namespaces is allowed.

[danehans]

@ironcladlou as @bowei mentions in #12 (comment), gateways should be able to bind routes from different namespaces. I'm going to update the Routes type to ObejctReference.

[danehans]

Some users may prefer not to use labels. In this case, should RouteSelector also support:

Routes []core.TypedLocalObjectReference `json:"routes"`

In this case, label-based selectors will be ignored. Thoughts?

[danehans]

Commit deb7775 adds Routes to RouteSelector to address the use case described in #12 (comment).

[ironcladlou]

This seems like an important difference, as you appear to be proposing the ability to admit routes to the gateway outside the gateway's namespace. My understanding of the current API is that only routes within the gateway's namespace can be admitted into that gateway. Is my understanding of the current API correct?

[ironcladlou]

Looking back over the the API sketch, Routes can be admitted to multiple gateways through route delegation and exporting, but it doesn't seem like namespaced references can be achieved directly with the Gateway->Route association. If true, I'm not sure if it's intentional.

The thing that caught my eye is the API this change is replacing seems to be scoped exclusively to local references, and the change would thus expand the scope to include namespaced references.

[danehans]

@ironcladlou my understanding is that a gateway supports route binding from namespaces that differ from the gateway's namespace. See the gateway config sample that binds routes from 2 different namespaces to gateway "gateway-sample". The namespaces of both routes differ from the gateway namespace.

[bowei]

Yes -- at least in the sketch (and we should make it explict), the idea is that gateway <=> route /can/ cross namespaces, to meet the gateway admin/user app separation.

I will add this to the use cases doc

[ironcladlou]

@bowei

Thanks for the clarification of intent. Looks like there's a possible mismatch here:

https://github.com/kubernetes-sigs/service-apis/blob/master/api/v1alpha1/gateway_types.go#L61

https://github.com/kubernetes-sigs/service-apis/blob/master/config/samples/networking_v1alpha1_gateway.yaml#L32

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/#typedlocalobjectreference-v1-core

Looks like maybe ObejctReference would be appropriate instead of TypedLocalObjectReference?

[danehans]

Commit e0637e3 updated the Route type to ObejctReference and the associated crd.

[danehans]

Fixes Issue: #53

[danehans]

Should Routes be broken out to specific routes, i.e. HTTPRouteRefs? The use of ObjectReference is discouraged. See #50 for details.

[bowei]

/assign bowei

[Miciah]

#86 proposes a change to route status that complements this change.

[danehans]

TODO: sketch out rbac permissions and change to default close if unspecified.

[danehans]

and associated ClusterRole definitions (yaml format).

@bowei let me know if this is what you were looking for regarding the permissions model for this feature.

[hbagdi]

Reading all the doc comments, I'm concluding that if nothing is defined, then the behavior is same as Ingress v1, i.e., this gateway associates all the routes in all namespaces.

Can we explicitly mention this to avoid confusion down the road?

[danehans]

@hbagdi I stuck with the previous defaulting behavior of Routes, meaning no routes are associated to a gateway if NamespaceSelector and RouteSelector are undefined. Per feedback from @bowei during last week's meeting, I'm having the behavior of these newly introduced properties match the behavior of metav1.LabelSelector.

[hbagdi]

I had a comment in the security doc that I'll re-iterate here: https://docs.google.com/document/d/14-mDATLLiF70wFaUPDXxFjO9BDrl9863Lk8um2RaTLs/edit

The labelSelector seems to be against the original motivation for adding namespace based selection in the first place.

As far as I understand, the original motivation for adding namespaceSelector was to make app devs ask for an explicit permission from the cluster operator before a route from a namespace that the app dev controls can be attached to a Gateway.

If instead a labelSelector is added, now the association of Gateway and Route is at the mercy of whoever has write access to the namespaces. Doesn't it defeat the purpose of namespaceSelector itself?

[danehans]

@hbagdi here's the use case behind RouteSelector:

• I have a single namespace that is shared among multiple dev teams containing a ton of routes.
• I have multiple gateways.
• I need the ability to associate different routes from the shared namespace to different gateways.

Thoughts on how to achieve this use case?

[hbagdi]

I'm all in for label selector for routes. I'm trying to understand the reasoning behind label selector for namespaces.

[M00nF1sh]

how do we handle priority between route? or routes is defined to be independent?
e.g. /path route and /* route have priority requirements to work correctly

[danehans]

RouteSelector is a selector for associating routes to a gateway. The following example associates route "foo" to gateway "my-gateway" but not route "bar":

kind: Gateway
apiVersion: networking.x.k8s.io/v1alpha1
metadata:
  name: my-gateway
  namespace: default
spec:
  class: acme-lb
  listeners:  # Use GatewayClass defaults for listener definition.
  - name: my-http-listener
    protocol: HTTP
  routes:
    routeSelector:
      matchLabels:
        app: foo
---
kind: HTTPRoute
apiVersion: networking.x.k8s.io/v1alpha1
metadata:
  name: foo
  namespace: default
  labels:
    app: foo
---
kind: HTTPRoute
apiVersion: networking.x.k8s.io/v1alpha1
metadata:
  name: bar
  namespace: default
  labels:
    app: bar

[bowei]

yes, is there an issue already regarding conflict resolution, should link it here.

[robscott]

Thanks for updating this @danehans! This matches what we'd decided on yesterday in the Service APIs meeting. Thanks for all the work to push this forward!

/lgtm

[bowei]

/lgtm

[bowei]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bowei, danehans

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [bowei]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rshriram]

why not just specify the namespace itself?

[danehans]

We decided to follow the established k8s pattern of using selectors. We may add additional RouteBindingSelector properties in the future. Feel free to create an issue to capture your use case.