knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.57k stars 1.16k forks source link

Domainmapping support dir-path-based routing #11997

Open jwcesign opened 3 years ago

jwcesign commented 3 years ago

Describe the feature

now it's little complex to config routing different dir-path to different ksvc, istio VS should be configured. such as https://github.com/knative/serving/issues/11892, So I think it's better to config this with domainmapping, like bellows(POC demo coding now): domaincliam.yaml

root@jw [04:18:19 PM] [~jw/testyaml]
-> # cat domainclaim.yaml
apiVersion: networking.internal.knative.dev/v1alpha1
kind: ClusterDomainClaim
metadata:
  name: test1.com
spec:
  namespace: default

domainmapping.yaml

root@jw [04:18:19 PM] [~jw/testyaml]
-> # cat domainmapping.yaml
apiVersion: serving.knative.dev/v1alpha1
kind: DomainMapping
metadata:
  name: test1.com
  namespace: default
spec:
  ref:
    - match:
      - uri:
        prefix: "/"
        name: home-page
        kind: Service
        apiVersion: serving.knative.dev/v1
    - match:
      - uri:
        prefix: "/search"
        name: search-go
        kind: Service
        apiVersion: serving.knative.dev/v1
    - match:
      - uri:
        prefix: "/login"
        name: login-go
        kind: Service
        apiVersion: serving.knative.dev/v1

This feature maybe just can be done based on istio gateway. Still not deeply check whether other gateways support.

/area API /assign @julz @mattmoor what do u think of this?

mattmoor commented 3 years ago

I prototyped a separate resource for this when we were getting started with DomainMapping, which I called Dispatcher. There is a little bit of a sketch in the team drive here: https://docs.google.com/document/d/1s5SS7XMupI37YQSXccPB2mcyNainjpEVhj-b8UEAfZ4/edit

I can probably find the branch where it lives if you care, but kingress does support path-based routing already today.

jwcesign commented 3 years ago

Thanks for answering, @mattmoor So what's the plan for "Dispatcher" feature? I check the code of ingress serving/vendor/knative.dev/networking/pkg/apis/networking/v1alpha1/ingress_types.go: HTTPIngressPath type, it has path-based-rule, so u mean the code is useless now?

mattmoor commented 3 years ago

That code was added to support the paths needed for ACME HTTP01 challenges, so it's used by our various auto-TLS integrations. So it works, but we don't expose a way for folks to configure it for themselves, yet.

AFAIK nobody is actively working on the Dispatcher concept, but I think it'd be a useful feature to complement Service and DomainMapping that lets folks compose a set of smaller services/functions into an API. My goal for Dispatcher was also to support Header and Method-based dispatch, which would take plumbing we don't currently support.

I'd defer to @julz and @dprotaso for the current direction of things, I mostly included the doc for historical context.

jwcesign commented 3 years ago

I check the doc, looks there is conflict with domainmapping? because domainmapping has only one backend service, and Dispatcher has many backend service. Maybe should combine them together? For kingress, I think it's not difficult to make it support VS(which support path-based routing).

mattmoor commented 3 years ago

The intent is to enable composition, but there was some discussion of combining them as well (back then).

I can't say I feel strongly (anymore, not that I have a vote anymore either!), but FWIW, regardless of how it manifests, I do think having path (and method/header) based routing would be yet-another killer feature for Knative, so if you take away anything from my Dispatcher digression, it should be: +1000 from me!

julz commented 3 years ago

I'm pretty +1 to landing something along the lines of Dispatcher also. Re: extending DomainMapping vs a separate Dispatcher etc.. I also find it a struggle to generate a strong opinion 😅. DomainMappings need the ClusterDomainClaim stuff to avoid collisions in the global namespace for domains, which might be a good reason to favour composition rather than extension to avoid Dispatcher having to worry about that too.

I think the main thing we need here (assuming @dprotaso is generally +1 too) is willing hands to write a feature proposal and actually do the work. @jwcesign do you have time/interest in driving this?

ZhiminXiang commented 3 years ago

my slight preference is to extend DomainMapping because of the existing ClusterDomainClaim mechanism as julz mentioned.

Looks like the gateway API supports path routing https://kubernetes.io/blog/2021/04/22/evolving-kubernetes-networking-with-the-gateway-api/#getting-hands-on-with-the-gateway-api. So Kingress should be good to add this feature. cc @nak3

mattmoor commented 3 years ago

@ZhiminXiang I think @julz was saying the opposite 😅 🤔

What I had in mind for the Dispatcher was to follow a similar convention to Route and to use a service in the namespace to give it foo.bar.svc.clusterl.local as the namespaced-"claim". DomainMapping could then be composed with it to get vanity URLs.

Not sure if @n3wscott ever landed his sugar where you could simply annotate Addressables to automagically spawn DomainMappings, but that should naturally extend auto-DM to Dispatcher as well.

julz commented 3 years ago

What I had in mind for the Dispatcher was to follow a similar convention to Route and to use a service in the namespace to give it foo.bar.svc.clusterl.local as the namespaced-"claim". DomainMapping could then be composed with it to get vanity URLs.

Yeah this was indeed the approach I was clumsily trying to express support for above :-)

jwcesign commented 3 years ago

I think the main thing we need here (assuming @dprotaso is generally +1 too) is willing hands to write a feature proposal and actually do the work. @jwcesign do you have time/interest in driving this?

Yes, very willing to drive this. So what I understand the talking is like bellows: domainmapping.yaml

apiVersion: serving.knative.dev/v1alpha1
kind: DomainMapping
metadata:
  name: test1.com
  namespace: default
spec:
  ref:
    name: backend-dispatcher 
    kind: Dispatcher
    apiVersion: serving.knative.dev/v1

dispatcher.yaml

apiVersion: serving.knative.dev/v1
kind: Dispatcher
metadata:
  name: backend-dispatcher
spec:
  backends:
  - match:
      path: /search
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: search

  - match:
      path: /login
    ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: login

  - ref:
      apiVersion: serving.knative.dev/v1
      kind: Service
      name: home

I understand right? @julz @mattmoor

mattmoor commented 3 years ago

It is beautiful! 👍

The neat thing about this is that as we build up more Addressable things, they can slot in where it'd currently mostly ksvc.

For example, in principal you could even layer Dispatcher resources if you wanted.

jwcesign commented 3 years ago

Hi, @mattmoor, would u mind telling me the branch of Dispatcher?

mattmoor commented 3 years ago

I poked around on my fork a tiny bit, but no obvious branch names popped out at me (and unfortunately I have loads).

If anyone knows of a good way to string search an entire Git repo (all refs), I'm happy to try that 😁

jwcesign commented 3 years ago

/assign @jwcesign

jwcesign commented 3 years ago

List of works:

emaildanwilson commented 3 years ago

Can a dispatcher route to ksvc's in multiple namespaces? It would be nice to have namespace in the ref section if possible.

mattmoor commented 3 years ago

kingress does not support this today (somewhat intentionally). I wonder whether Gateway APIs allows for this, since Ingress did not.

emaildanwilson commented 3 years ago

I have not looked at the API entirely but it looks like it does support multiple namespaces. https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.RouteNamespaces

jwcesign commented 3 years ago

because of security problem, I prefer dispathcer only works in one namespace.

jwcesign commented 3 years ago

https://docs.google.com/document/d/1q3kkBhSqWMHm-TCFo56R-32C7-fo6G8KQH4lIGtc-U0/edit?usp=sharing here is the first proposal, If the way is right, I will do more details design. cc @julz @mattmoor

lizzzcai commented 3 years ago

Hi @jwcesign @mattmoor, I noticed this path-based routing and I think it is very useful and an add-on feature on the current host-based routing. I had gone through this Dispatcher but I am not so sure if I can solve my problem especially when it is can only route to service in the same namespace. And wondering if there is any easy and direct method to support the path-based routing in knative.

Maybe I will describe my use cases here and you can help to answer if this can be solved. (if there is a way to solve it in knative already, please let me know, thanks)

We have deployed some services by kfserving/knative. for example, we have the following namespaces:

above are two namespaces but they are belonging to the same user (user1, so the services are crossing multiple namespaces).

within each namespace, we have two servings, service1 and service 2. In current host-based routing, we have the following deployment URLs:

What we expect is a path-based routing like the following:

api.example.com/user1-ns1/service1 api.example.com/user1-ns1/service2 api.example.com/user1-ns2/service1 api.example.com/user1-ns2/service2

or user1-ns1.example.com/service1 user1-ns2.example.com/service1

etc.

when user calls a path foo, it will be like service1.user1-ns1.example.com/foo in host-based or api.example.com/user1-ns1/service1/foo in path-based.

we are expecting the host to be fixed as much as possible and only the path is changing based on the service.

currently we do it by adding a virtual service to route the path-based URL(api.example.com/user1-ns1/service1) to the service (service1.user1-ns1.svc.cluster.local).

for example, for api.example.com/user1-ns1/service1, the following vs is applied.

apiVersion: networking.istio.io/v1beta1
kind: VirtualService
metadata:
  name: service1-path-based-endpoint
  namespace: user1-ns1
spec:
  gateways:
  - istio-system/ingress-gateway
  hosts:
  - api.example.com
  http:
  - name: "service1-path-based-routes"
    match:
    - uri:
        prefix: "/user1-ns1/service1/"
      ignoreUriCase: true
    rewrite:
      uri: "/"
    route:
    - destination:
        host: local-gateway.istio-system.svc.cluster.local
        port:
          number: 80
      headers:
        request:
          set:
            Host: service1.user1-ns1.svc.cluster.local
      weight: 100

currently, the root-based URL is controlled by the domainTemplate.

domainTemplate: {{` "{{.Name}}-{{.Namespace}}.{{.Domain}}" `}}

I am wondering if it is possible to enable the path-based endpoint in a similar way by defining a template and turning on the feature. like:

domainTemplate: {{` "{{.Name}}-{{.Namespace}}.{{.Domain}}" `}}
pathBasedRoutingEnabled: True
pathTemplate: {{` "api.{{.Domain}}/{{.Namespace}}/{{.Name}}" `}} # user need to avoid conflict of the path by themselves.

That will be easier to support the use cases I mentioned above, and avoid additional steps to create and update the dispatcher when a new service comes in. Of cause on top of this feature, the dispatcher can be an add-on for the more complicated use cases.

jwcesign commented 3 years ago

hi, @lizzzcai, the only way to support path based routing is using VirtualService for now(like what u do now).

I am wondering whether to support routing to multi-namespace. But there is a problem. Dispatcher will be developed based on VirtualService, but VirtualService has ns limitation.

lizzzcai commented 3 years ago

Hi @jwcesign, thanks for your reply.

is it possible to support path-based routing as a feature natively? like what I mentioned above (user turn on the feature and provide a template). Or another way to support it like a pathRoutingMapping kind of feature (provide a template format then create the vs by knative)

like you said, there are many similar requests:

and there is a document to support this as well: https://github.com/knative/docs/tree/main/docs/serving/samples/knative-routing-go.

I think there will be a lot of requests to support routing to service in multi-namespace, like the use case I mentioned above, we may have a main tenant which owns resource groups(multi namespace). So the domain of this main tenant can route to services under different resource groups, and we can use the authorization policy to ensure security.

And for the dispatcher, if I want to add a new service, I need to patch the dispatcher (I would expect no downtime occurs). Is there a way to dynamically add the new service into the dispatcher? (like a select *(service) and apply a template)

jwcesign commented 3 years ago

cc @mattmoor @julz

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

simonkrenger commented 2 years ago

/remove-lifecycle stale

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

simonkrenger commented 2 years ago

/reopen /remove-lifecycle stale

lookis commented 2 years ago

hello, any update on this? @julz @mattmoor @jwcesign @lizzzcai

jwcesign commented 2 years ago

@lookis Still in proposal, what is your usecase?

lookis commented 2 years ago

@jwcesign

exactly the same. I have to use istio VS to route the path for now, but VS doesn't support tls, istio try to support tls under Gateway, DomainMapping support tls, but do not support path routing.

I prefer knative way to deal with it, because I am in a multi tanent situation, which tls cert should belong to each namespace separately.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

simonkrenger commented 2 years ago

/remove-lifecycle stale /reopen

github-actions[bot] commented 1 year ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.

simonkrenger commented 1 year ago

/remove-lifecycle stale /reopen

whatnick commented 1 year ago

This of interest to me as well particularly for applications behind ALB + Cognito. Without path based routing the application clients need to be configured with multiple redirect URI's which have a hard limit of 100. This limits the scalability of shared application clients across multiple knative applications and poses operational challenges.

vhico07 commented 1 year ago

Are there any updates regarding the route path in DomainMapping? I've implemented Knative in staging at about 90% completion, but I'm encountering issues with the route path. It would be unfortunate if the project were halted just because of this challenge.

dprotaso commented 1 year ago

/unassign @jwcesign

I don't believe anyone is working on this.

If you're using Istio a workaround would be programming a VirtualService - https://github.com/knative/docs/blob/main/code-samples/serving/knative-routing-go/routing-internal.yaml

Likewise for Contour you'd create a similar HTTPProxy resource