serving icon indicating copy to clipboard operation
serving copied to clipboard

Domainmapping support dir-path-based routing

Open jwcesign opened this issue 4 years ago • 33 comments
trafficstars

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?

jwcesign avatar Sep 15 '21 03:09 jwcesign

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.

mattmoor avatar Sep 15 '21 03:09 mattmoor

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?

jwcesign avatar Sep 15 '21 03:09 jwcesign

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.

mattmoor avatar Sep 15 '21 04:09 mattmoor

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).

jwcesign avatar Sep 15 '21 07:09 jwcesign

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!

mattmoor avatar Sep 15 '21 17:09 mattmoor

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?

julz avatar Sep 15 '21 20:09 julz

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

ZhiminXiang avatar Sep 15 '21 20:09 ZhiminXiang

@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.

mattmoor avatar Sep 15 '21 21:09 mattmoor

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 :-)

julz avatar Sep 15 '21 21:09 julz

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

jwcesign avatar Sep 16 '21 08:09 jwcesign

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.

mattmoor avatar Sep 16 '21 16:09 mattmoor

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

jwcesign avatar Sep 21 '21 05:09 jwcesign

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 😁

mattmoor avatar Sep 21 '21 15:09 mattmoor

/assign @jwcesign

jwcesign avatar Sep 26 '21 09:09 jwcesign

List of works:

  • [ ] Writing feature proposal

jwcesign avatar Sep 26 '21 09:09 jwcesign

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

emaildanwilson avatar Sep 28 '21 17:09 emaildanwilson

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

mattmoor avatar Sep 28 '21 17:09 mattmoor

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

emaildanwilson avatar Sep 28 '21 17:09 emaildanwilson

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

jwcesign avatar Oct 24 '21 12:10 jwcesign

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

jwcesign avatar Oct 26 '21 09:10 jwcesign

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:

  • user1-ns1
  • user1-ns2

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:

  • service1.user1-ns1.example.com
  • service2.user1-ns1.example.com
  • service1.user1-ns2.example.com
  • service2.user1-ns2.example.com

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.

lizzzcai avatar Oct 27 '21 17:10 lizzzcai

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.

jwcesign avatar Oct 28 '21 10:10 jwcesign

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:

  • https://github.com/knative/serving/issues/540#issuecomment-382562663
  • https://github.com/knative/serving/issues/9156
  • https://github.com/knative/serving/issues/11892

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)

lizzzcai avatar Oct 28 '21 11:10 lizzzcai

cc @mattmoor @julz

jwcesign avatar Oct 28 '21 12:10 jwcesign

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.

github-actions[bot] avatar Jan 27 '22 01:01 github-actions[bot]

/remove-lifecycle stale

simonkrenger avatar Jan 27 '22 07:01 simonkrenger

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.

github-actions[bot] avatar Apr 28 '22 01:04 github-actions[bot]

/reopen /remove-lifecycle stale

simonkrenger avatar Apr 28 '22 09:04 simonkrenger

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

lookis avatar May 27 '22 22:05 lookis

@lookis Still in proposal, what is your usecase?

jwcesign avatar May 30 '22 01:05 jwcesign