Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Domainmapping support dir-path-based routing #11997

Open
jwcesign opened this issue Sep 15, 2021 · 38 comments
Open

Domainmapping support dir-path-based routing #11997

jwcesign opened this issue Sep 15, 2021 · 38 comments
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding. triage/accepted Issues which should be fixed (post-triage)
Milestone

Comments

@jwcesign
Copy link
Member

jwcesign commented Sep 15, 2021

Describe the feature

now it's little complex to config routing different dir-path to different ksvc, istio VS should be configured. such as #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 jwcesign added the kind/feature Well-understood/specified features, ready for coding. label Sep 15, 2021
@knative-prow-robot knative-prow-robot added the area/API API objects and controllers label Sep 15, 2021
@mattmoor
Copy link
Member

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
Copy link
Member Author

jwcesign commented Sep 15, 2021

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
Copy link
Member

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
Copy link
Member Author

jwcesign commented Sep 15, 2021

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
Copy link
Member

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
Copy link
Member

julz commented Sep 15, 2021

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
Copy link

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
Copy link
Member

@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
Copy link
Member

julz commented Sep 15, 2021

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
Copy link
Member Author

jwcesign commented Sep 16, 2021

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
Copy link
Member

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
Copy link
Member Author

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

@mattmoor
Copy link
Member

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
Copy link
Member Author

/assign @jwcesign

@jwcesign
Copy link
Member Author

jwcesign commented Sep 26, 2021

List of works:

  • Writing feature proposal

@emaildanwilson
Copy link
Contributor

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

@mattmoor
Copy link
Member

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

@emaildanwilson
Copy link
Contributor

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
Copy link
Member Author

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

@jwcesign
Copy link
Member Author

jwcesign commented Oct 26, 2021

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
Copy link

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.

@jwcesign
Copy link
Member Author

jwcesign commented Oct 28, 2021

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
Copy link

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
Copy link
Member Author

cc @mattmoor @julz

@github-actions
Copy link

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 27, 2022
@simonkrenger
Copy link

/remove-lifecycle stale

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 27, 2022
@github-actions
Copy link

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 28, 2022
@simonkrenger
Copy link

/reopen
/remove-lifecycle stale

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 28, 2022
@lookis
Copy link

lookis commented May 27, 2022

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

@jwcesign
Copy link
Member Author

jwcesign commented May 30, 2022

@lookis Still in proposal, what is your usecase?

@lookis
Copy link

lookis commented May 30, 2022

@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
Copy link

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 29, 2022
@simonkrenger
Copy link

/remove-lifecycle stale
/reopen

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 29, 2022
@github-actions
Copy link

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 github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 29, 2022
@simonkrenger
Copy link

/remove-lifecycle stale
/reopen

@knative-prow knative-prow bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 29, 2022
@ReToCode ReToCode added the triage/accepted Issues which should be fixed (post-triage) label Mar 8, 2023
@whatnick
Copy link

whatnick commented Aug 8, 2023

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
Copy link

vhico07 commented Aug 29, 2023

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
Copy link
Member

/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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding. triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Development

No branches or pull requests