-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Dynamic Routing Error: Using dynamic routing with services that use HTTP and HTTPS #2253
Comments
Something I forgot to mention is that the routes with HTTP are resolved successfully and if I want to make requests to the routes with HTTPS it returns an error and it is because the route resolution is HTTP and not HTTPS. |
Could you make the title shorter please? Also, I'm confused in recognizing this issue as a bug or an question? |
@viktoralfa19 Are you online? |
Victor, {
"DynamicRoutes": [
{
"ServiceName": "openloyalti",
"DownstreamScheme": "https" // important! It is HTTPS route
}
],
"GlobalConfiguration": {
"ServiceDiscoveryProvider": {
// ...
},
"RateLimitOptions": {
// ...
},
// ...
"DownstreamScheme": "http" // important! ALL routes are HTTP
}
} Am I correct? 🆗 This feature is not currently implemented in Ocelot! All dynamic routes should be HTTP or HTTPS globally, as defined in the Explanation of current design: File-models
Finally, global properties from |
Please upgrade to the latest stable version 23.4.3❗
As I have explained above, it is now impossible to override the scheme in route JSON via 🆗 To draft a solution, we need to override the Ocelot/src/Ocelot.Provider.Consul/DefaultConsulServiceBuilder.cs Lines 91 to 94 in 1b34be9
This method currently calls the constructor with two arguments: Ocelot/src/Ocelot/Values/ServiceHostAndPort.cs Lines 12 to 16 in 1b34be9
To reapply the actual scheme, we must call the constructor with three arguments, including the scheme: Ocelot/src/Ocelot/Values/ServiceHostAndPort.cs Lines 18 to 19 in 1b34be9
The remaining task is to attach the new behavior class utilizing the AddConsul<T> method. Which approach do you prefer: JSON or C#? |
Thank you very much for answering quickly. Of course nice to collaborate |
I can't update to the current version 23.4.3 because I have another bug that I'm going to upload with a description |
I would like to implement the modification made with C# code but it cannot be implemented by extending the functionality of the ocelot.consul library, or should it be changed in its source code? |
@raman-m public class SchemaConsulServiceBuilder(
IHttpContextAccessor contextAccessor,
IConsulClientFactory clientFactory,
IOcelotLoggerFactory loggerFactory)
: DefaultConsulServiceBuilder(contextAccessor, clientFactory, loggerFactory)
{
protected override string GetDownstreamHost(ServiceEntry entry, Node node) => entry.Service.Address;
protected override ServiceHostAndPort GetServiceHostAndPort(ServiceEntry entry, Node node)
{
if (entry.Service.Meta?.TryGetValue("Scheme", out var schema) ?? false)
return new ServiceHostAndPort(GetDownstreamHost(entry, node), entry.Service.Port,
schema);
return new ServiceHostAndPort(GetDownstreamHost(entry, node), entry.Service.Port);
}
} And I used it here: webApplicationBuilder.Services.AddOcelot(configuration).AddConsul<SchemaConsulServiceBuilder>(); I hope this helps someone else. |
Indeed, the
What is notable in your code is the extraction of the scheme value: entry.Service.Meta?.TryGetValue("Scheme", out var schema) This is the cornerstone of the logic. The agent service, known Could you describe this use case in detail, please? I have an idea to work more on this case and possibly create a PR together with you. |
Sorry @raman-m , due to time constraints I was not able to respond in time. Regarding your use case question, the truth is very simple at least until now:
Currently, together with my work team, we are developing a product based on microservices, where we need to centralize not only the APIs of our internal services but also components that provide services to us internally such as our own Identity Server or a CSM and also the APIs of external services. For clients, we coordinate actions between all these components, being completely agnostic to them. That is why the use of Consul as Service Discovery and as ApiGateway Ocelot work very well together, we are still in iterations of the solution, but we have reached the point that if we decide to use dynamic routing we will have the need to route from Ocelot to the endpoint that they use http which are our internal components and the external services which use https, hence the need to make a PoC to know if a scenario like this was feasible or not. The definition we use when creating our external services within Consul is the following:
In this way we load the configuration of the external services in Consul when raising the container. Obviously we didn't have the scheme before, but with the solution you gave me and analyzing in debug how the registry arrives from Consul, it occurred to me to send it as part of the Metadata of the registry.
Well, for my part, I would be happy to listen to the proposal since I am interested in knowing more about Ocelot since it is going to be a component that will go by default in other projects that we have. |
Very well, Victor! I will propose something by opening a PR, but only after the .NET 9 release. My idea involves enhancing the existing DefaultConsulServiceBuilder class. We have a similar feature in Kubernetes provider: Downstream Scheme vs Port Names. |
Please I have a simple .Net 8 web api application where Ocelot version 23.2.2 is used and I have this configuration. I am also using CONSUL as service discovery.
When running in a container with docker-compose and trying to use dynamic routing I want to use internal APIs with HTTP and external APIs with HTTPS so I configure OCELOT like this:
When doing so I always get this exception:
Basically I want to know how to solve my problem of using dynamic routing with services that use HTTP and other services that use HTTPS.
The text was updated successfully, but these errors were encountered: