gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

Support namespace scoped implementations

Open howardjohn opened this issue 4 years ago • 20 comments

What would you like to be added:

Currently, someone deploying an implementation must have cluster scoped privileges to access GatewayClass. Often in multitenant clusters, teams are given access only to a set of namespaces without permissions to access cluster scoped resources. Users in this scenario should be able to deploy an implementation of the API.

For example, they would deploy an nginx proxy controller, a LoadBalancer Service, a Gateway, and some routes. This is really close to working, but they still need to create some GatewayClass.

Potential options

  1. Cluster admin creates generic GatewayClass with no params, say "in-cluster-proxy". NS admin creates a Gateway referencing it. They still need to configure parameters of the gateway (ie things that would typically exist in parametersRef of Gatewayclass), which they cannot do in GatewayClass. As a result, they need to externalize these configurations to somewhere else (annotations on the deployment/service/gateway, nginx-specific configmap, etc). This isn't great since now its entirely implementation specific how things are configured - but its also not too bad, as parametersRef is all implementation specific anyways. Still requires some cluster admin coordination

  2. Same as (1), but maybe they don't even bother with a generic GatewayClass and just put some bogus value there (it is a required field)

  3. Do not support deployments with only namespace permissions

howardjohn avatar Mar 01 '21 22:03 howardjohn

cc @costinm

howardjohn avatar Mar 01 '21 22:03 howardjohn

We discussed this at the last community meeting. It sounded like there was significant interest in deployment models without the GatewayClass resource. Maybe something that could look like this:

  1. Each Gateway implementation can choose to support a set of static, domain-prefixed classes (such as gke.io/xlb). These would not require GatewayClass resources to exist.
  2. Users could specify those classes in the same way they specified any other class on a Gateway resource, there just wouldn't be a GatewayClass resource.
  3. GatewayClass resources matching these static names would be ignored by the implementation with the exception of potentially adding some kind of status indicating the conflict with the static class.

/cc @danehans @hbagdi @bowei

robscott avatar Mar 10 '21 06:03 robscott

While I agree with you @robscott (for the need of namespace scoped installations), I feel the same issue is similiar to the normal IngressClass Ressource, no? As this is also a cluster scoped ressource. So maybe its possible to find a solution that works for both sides?

re: https://github.com/kubernetes/kubernetes/issues/99824

I feel like, we should try to solve that all together? 🤷‍♂️

SantoDE avatar Mar 10 '21 10:03 SantoDE

Each Gateway implementation can choose to support a set of static, domain-prefixed classes (such as gke.io/xlb)

We had mentioned before the GatewayClass is optional, but if its added it will be used (to use parameters in the GWC). If we use this syntax I don't think its possible since you cannot have / in the name? I guess your third comment is conflicting with that, so maybe that was just my impression that wasn't shared by others. I think its useful to be able to overwrite it.

howardjohn avatar Mar 10 '21 16:03 howardjohn

Thanks @SantoDE for linking this to the upstream issue! I agree that whatever we should try to gain consensus in the broader sig-network community as well to ensure any change can help with Ingress as well as Gateway.

You're completely right @howardjohn, the example I gave of gke.io/xlb wouldn't work. My goal was to define some patter that would make conflicts unlikely or impossible. For example, it would be bad if 2 controllers both claimed the xlb class if there was no formal GatewayClass definition. Although we could use some kind of subdomain format, that seems rather unnatural. Maybe the format should simply be {implementation-name}-{variation}. So to follow up with my example above, gke-xlb. That seems rather unlikely to conflict with anything, but there are others that may be trickier. IE there is more than one nginx ingress implementation.

I think its useful to be able to overwrite it.

I'm not completely tied to the idea that GatewayClasses of these names would be invalid/ignored, but I still prefer it. If implementations were to provide standard classes with internally consistent meanings, it could get confusing if that class name meant something slightly different in another cluster. Maybe more significantly, if a GatewayClass resource could be used to override the meaning of one of these static class names, every controller would have to be able to read from the cluster-scoped GatewayClass API, which would be a significant hurdle for namespace-scoped deployments.

robscott avatar Mar 12 '21 06:03 robscott

Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale

fejta-bot avatar Jun 10 '21 06:06 fejta-bot

/remove-lifecycle stale /lifecyce frozen

jpeach avatar Jul 01 '21 22:07 jpeach

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot avatar Sep 29 '21 23:09 k8s-triage-robot

/lifecycle frozen

hbagdi avatar Sep 30 '21 21:09 hbagdi

I think our best solution here would be to provide implementations with a pattern they can follow if they want to support single-namespace deployments. For example, implementations could provide a flag to indicate that they should only watch resources within the namespace they are deployed along with a flag to indicate which the class name they should implement. This class name should not overlap with any cluster-wide class names to avoid multiple implementations trying to implement the same resource.

robscott avatar Aug 04 '22 22:08 robscott

I think our best solution here would be to provide implementations with a pattern they can follow if they want to support single-namespace deployments. For example, implementations could provide a flag to indicate that they should only watch resources within the namespace they are deployed along with a flag to indicate which the class name they should implement. This class name should not overlap with any cluster-wide class names to avoid multiple implementations trying to implement the same resource.

Maybe we should revisit https://github.com/kubernetes-sigs/gateway-api/issues/136? It still strikes me as the most elegant solution. It is not about managing infra, but about pointing to infra within the multi-tenancy world in which we live on Kubernetes. Without a proper ClusterGatewayClass and GatewayClass, I argue everything else is a work-around to the most correct solution.

Regarding your proposal @robscott, I'm not sure how that is diffrent than what GWC used to have back in early days?

https://github.com/kubernetes-sigs/gateway-api/blob/fd5ffd1db31fb9a5b6dff1010679bcf42777738d/apis/v1alpha1/gatewayclass_types.go#L67-L84

It kind of feels like this has come full circle...

akutz avatar Sep 23 '22 21:09 akutz

There are a few problems I can see with introducing a ClusterGatewayClass and associated GatewayClass namespace-scoped resource:

  • *Class names are, in the Kubernetes API, generally reserved for cluster-scoped things. IngressClass and StorageClass are both cluster scoped. So I'd probably be more in favor of keeping GatewayClass and introducing NamespacedGatewayClass or something.
  • Once we do have a namespaced reference to a gateway bucket (I'll go with NamespacedGatewayClass for the purposes of this argument), we have to determine how it works.
    • Do we add a namespace field to the spec.GatewayClassName of Gateway? This would mean moving this field to be a struct with a name and namespace reference, and using the convention that "" means "cluster-scoped". That convention is used elsewhere in the API, so that part is fine, but this is a big spec change.
    • Once we have a namespace reference, we also need to consider safe cross-namespace references, and use the ReferenceGrant-style functionality either by using ReferenceGrant directly, or adding an AllowedNamespaces field to NamespacedGatewayClass as @akutz suggests above. Again, this is a pretty significant departure.
    • If we allow this to go outside a single namespace, we're not saving much in terms of controller access, since the controller will still need cluster-level access to Gateways, NamespacedGatewayClass, etc.
    • Alternatively, we could not change the spec of Gateway at all, and just say "first check the same namespace as the Gateway for a NamespacedGatewayClass of that name, and if so, use that, and then all references must be within that namespace." I guess that's doable, but would mean that you can't do a set of namespaces. And that's a lot of magic that would not be directly visible from the objects themselves.

I guess I'm saying that I don't agree that ClusterGatewayClass is the more correct option, and changing to it at this point would be an api-breaking change that would require v1beta2. I don't think providing the functionality in the top comment of the issue is worth the months of work it would take to do a v1beta2. We would need to do a full pass through v1alpha3 or something, then move to v1beta2.

To put this another way, I don't think there's a way to make compatible changes to the API to get us to a NamespacedGatewayClass at this point, and I don't know if this is a big enough issue for the whole community to spend the effort to roll the apiVersion. (I acknowledge that's not very satisfying for people who need a resolution).

I think that the workarounds proposed about not requiring an actual GatewayClass object to be present are probably the best we're going to be able to do without a new apiVersion.

youngnick avatar Sep 26 '22 04:09 youngnick

@howardjohn does this represent a known customer need for Istio (or any other implementations you work on) or is it more theoretical? If this is more of a theoretical need given the difficulties I feel slightly inclined to close it in favor of waiting for a time where the needs of end-users can move it forward. In either case do we feel comfortable with saying we wouldn't need this for GA, as we should be able to use the ObjectName reference in the current GatewaySpec pretty open-endedly? Let me know your thoughts :thinking:

shaneutt avatar Mar 14 '23 23:03 shaneutt

The main thing is params need to be namespace scoped. Having GatewayClass is not too bad if you just have 1 for the entire cluster, but per-namespace owners need to be able to deploy and configure Gateways. This is handled by https://github.com/kubernetes-sigs/gateway-api/pull/1757 though.

https://istio.io/latest/docs/setup/additional-setup/gateway/#dedicated-application-gateway is an example in Istio

howardjohn avatar Mar 14 '23 23:03 howardjohn

@shaneutt this is a very common use case, based on teams, tenancy, and ownership of apps/components within a single K8s cluster environment, there is a need to support a wide range of cases where

  • one team (gateway owned by app team ) would like to only watch for resources in select namespaces
  • another team (maybe a common platform team) would like to watch resources in all namespaces

Implementations can solve this by adding a notion of deployment mode to decide which namespaces to watch resources in and which namespaces to deploy managed infra in, but since GatewayClass is cluster scoped / not namespace scoped, we have to trust users (without any formal restriction) creating the GatewayClass to link it to the right implementation controller, (rather than the intent and capability of the controller itself to make this call)

arkodg avatar Mar 14 '23 23:03 arkodg

Good, makes sense to me but seeing active support for this gives me extra confidence in prioritizing it sooner rather than later so let's move this out of triage then and consider it something we'd like to solve for v1.0.0.

shaneutt avatar Mar 15 '23 01:03 shaneutt

Definitely still wanted by many implements and is a common use case, but doesn't need to hold up GA: we should be able to add this functionality in a post-GA world without too much hassle.

shaneutt avatar Jul 10 '23 21:07 shaneutt

It seems like this still has obvious benefits to the ecosystem, despite the lack of anyone to champion it and drive it forward. Let's remove the frozen lifecycle from it at least so it can remind us that it's hanging out back here, and we can continue to re-evaluate.

shaneutt avatar Mar 12 '24 15:03 shaneutt