ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.31k stars 1.63k forks source link

Add a possibility to change service name dynamically because of Consul tags #1188

Open mantasaudickas opened 4 years ago

mantasaudickas commented 4 years ago

Hello,

At the moment service name is chosen based on first segment in the path. I would like to be able to map one (or multiple) path segments to service name in Consul.

Few examples: authorization => would mapped to authorization.service.web authorization/api => would be mapped to authorization.service.api

Reason for this, that my routes are defined in Consul tags and I would like to use them as a key when selecting service name in Consul. At the moment to achieve this I have to replace whole DownstreamRouteCreator class, while I would be keen to keep custom part as small as possible.

Would you mind if I make a PR for this?

So something like:

    public interface IServiceNameProvider
    {
        string GetServiceName(string upstreamUrlPath);
    }

    public class ServiceNameProvider: IServiceNameProvider
    {
        public string GetServiceName(string upstreamUrlPath)
        {
            if (upstreamUrlPath.IndexOf('/', 1) == -1)
            {
                return upstreamUrlPath
                    .Substring(1);
            }

            return upstreamUrlPath
                .Substring(1, upstreamUrlPath.IndexOf('/', 1))
                .TrimEnd('/');
        }
    }

and use this such provider in CustomDownstreamRouteCreator class.

with best regards

raman-m commented 1 year ago

Hello, Mantas! Thanks for your interest in Ocelot!

I see the problem... Do you have saved Consul configs or maybe screenshots of Consul dashboard?

authorization/api

Do you mean, an extra slash char starts Consul tag? Is /api a tag in Consul for the service name?

Finally, I see that complete Consul service name template is {servicename}/{tag}. Correct?

Could you share a link to Consul docs regarding tags please?

raman-m commented 1 year ago
+ Accepted

... because of ready PR #1198

mantasaudickas commented 1 year ago

Finally, I see that complete Consul service name template is {servicename}/{tag}. Correct?

Not really.. we can have multiple services behind gateway, for example: /authorization - points to angular project /authorization/api - points to independently deployed dotnet project /authorization/api/user - can point to a separate service which is providing user details... /authorization/jobs - might be some background job service which is executing some background business logic (like read model updates) and also provides some technical API endpoints

So as you can see url starts with the same word.. but further on we can have more microservices running. When routing - we simply sort these known urls by length and then choose most matching.

raman-m commented 1 year ago

OK I see your team uses path-style to define tags... In the example above authorization is like a DNS name, right? I found this Consul docs related to tags: tags | Service configuration reference | Consul | HashiCorp Developer This doc has these requirements for tag definition:

Tag values are opaque to Consul. We recommend using valid DNS labels for service definition IDs for compatibility with external DNSs.

But I think it is not a stopper for PR logic, because you have not introduced any Consul hardcoded features. Only feature decoupling by interface.

I thought Consul has some requirements and you wish to introduce them in the PR. But now I understand that you've introduced decoupling of service name getter logic, and then it will be possible to inject any custom service name getter including Consul one.

No questions anymore! 😸

mantasaudickas commented 1 year ago

In the example above authorization is like a DNS name, right?

Not really. Path is just a key to configuration item. That item has all necessary information about downstream service.

To be honest - now I would do this PR a bit differently. Finder should take HttpRequest as parameter and then you can have various logics in routing where service instance can be identified by:

In microservices world where services are created and registered kind of dynamically such kind of flexibility is necessary. In our infrastructure there are hundreds of microservices running and routing rules might vary quite a lot thus in application gateway a lot of flexibility is required :)

raman-m commented 1 year ago

@mantasaudickas commented on July 28:

To be honest - now I would do this PR a bit differently.

Any changes and improvements are welcome in current PR! I'll be happy to review...


Finder should take HttpRequest as parameter and then you can have various logics in routing where service instance can be identified by:

  • path prefix
  • query parameter
  • host name
  • request header
  • even port

Some of these properties we have in current Get() method of the IDownstreamRouteProvider interface 👇

public Response<DownstreamRouteHolder> Get(
    string upstreamUrlPath, 
    string upstreamQueryString, 
    string upstreamHttpMethod, 
    IInternalConfiguration configuration, 
    string upstreamHost)
{ }

So, any additional objects vs data will require to change the IDownstreamRouteProvider interface. Think twice before making such kind of code changes.

Will you push some commit soon?

mantasaudickas commented 1 year ago

Will you push some commit soon?

Sorry, no chance..

But you can do it on your own.. just IDownstreamServiceFinder should have this signature:

string GetServiceName(HttpRequest request)
raman-m commented 5 months ago

@ggnaegi Your opinion, Gui? Will this feature be useful for you and the community?

raman-m commented 2 months ago

@ggnaegi Seems it is not useful anymore after #2067 delivery.