Skip to content

Commit

Permalink
Fix issue ThreeMammals#936: Kubernetes service discovery provider doe…
Browse files Browse the repository at this point in the history
…sn't allow cross-namespace discovery (ThreeMammals#938)

* Allow default k8s namespace to be overridden

* Add ServiceNamespace to ReRoute configuration

* Remove debug comments

* Update unit tests

* Unit tests (Eureka)

* Update docs

* Re-run build
  • Loading branch information
jasongm86 authored and thiagoloureiro committed Jun 25, 2019
1 parent 959a92e commit e1d7f28
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 20 deletions.
18 changes: 18 additions & 0 deletions docs/features/kubernetes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,21 @@ The polling interval is in milliseconds and tells Ocelot how often to call kuber

Please note there are tradeoffs here. If you poll kubernetes it is possible Ocelot will not know if a service is down depending on your polling interval and you might get more errors than if you get the latest services per request. This really depends on how volatile your services are. I doubt it will matter for most people and polling may give a tiny performance improvement over calling kubernetes per request.
There is no way for Ocelot to work these out for you.

If your downstream service resides in a different namespace you can override the global setting at the ReRoute level by specifying a ServiceNamespace


.. code-block:: json
{
"ReRoutes": [
{
"DownstreamPathTemplate": "/api/values",
"DownstreamScheme": "http",
"UpstreamPathTemplate": "/values",
"ServiceName": "downstreamservice",
"ServiceNamespace": "downstream-namespace",
"UpstreamHttpMethod": [ "Get" ]
}
]
}
4 changes: 2 additions & 2 deletions src/Ocelot.Provider.Consul/ConsulProviderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@

public static class ConsulProviderFactory
{
public static ServiceDiscoveryFinderDelegate Get = (provider, config, name) =>
public static ServiceDiscoveryFinderDelegate Get = (provider, config, reRoute) =>
{
var factory = provider.GetService<IOcelotLoggerFactory>();

var consulFactory = provider.GetService<IConsulClientFactory>();

var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Host, config.Port, name, config.Token);
var consulRegistryConfiguration = new ConsulRegistryConfiguration(config.Host, config.Port, reRoute.ServiceName, config.Token);

var consulServiceDiscoveryProvider = new Consul(consulRegistryConfiguration, factory, consulFactory);

Expand Down
4 changes: 2 additions & 2 deletions src/Ocelot.Provider.Eureka/EurekaProviderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@

public static class EurekaProviderFactory
{
public static ServiceDiscoveryFinderDelegate Get = (provider, config, name) =>
public static ServiceDiscoveryFinderDelegate Get = (provider, config, reRoute) =>
{
var client = provider.GetService<IDiscoveryClient>();

if (config.Type?.ToLower() == "eureka" && client != null)
{
return new Eureka(name, client);
return new Eureka(reRoute.ServiceName, client);
}

return null;
Expand Down
3 changes: 1 addition & 2 deletions src/Ocelot.Provider.Kubernetes/KubeProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ public Kube(KubeRegistryConfiguration kubeRegistryConfiguration, IOcelotLoggerFa

public async Task<List<Service>> Get()
{
var service = await kubeApi.ServicesV1()
.Get(kubeRegistryConfiguration.KeyOfServiceInK8s, kubeRegistryConfiguration.KubeNamespace);
var service = await kubeApi.ServicesV1().Get(kubeRegistryConfiguration.KeyOfServiceInK8s, kubeRegistryConfiguration.KubeNamespace);
var services = new List<Service>();
if (IsValid(service))
{
Expand Down
11 changes: 6 additions & 5 deletions src/Ocelot.Provider.Kubernetes/KubernetesProviderFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,25 @@
using Ocelot.Logging;
using Ocelot.ServiceDiscovery;
using System;
using Ocelot.Configuration;

namespace Ocelot.Provider.Kubernetes
{
public static class KubernetesProviderFactory
{
public static ServiceDiscoveryFinderDelegate Get = (provider, config, name) =>
public static ServiceDiscoveryFinderDelegate Get = (provider, config, reRoute) =>
{
var factory = provider.GetService<IOcelotLoggerFactory>();
return GetkubeProvider(provider, config, name, factory);
return GetkubeProvider(provider, config, reRoute, factory);
};

private static ServiceDiscovery.Providers.IServiceDiscoveryProvider GetkubeProvider(IServiceProvider provider, Configuration.ServiceProviderConfiguration config, string name, IOcelotLoggerFactory factory)
private static ServiceDiscovery.Providers.IServiceDiscoveryProvider GetkubeProvider(IServiceProvider provider, Configuration.ServiceProviderConfiguration config, DownstreamReRoute reRoute, IOcelotLoggerFactory factory)
{
var kubeClient = provider.GetService<IKubeApiClient>();
var k8sRegistryConfiguration = new KubeRegistryConfiguration()
{
KeyOfServiceInK8s = name,
KubeNamespace = config.Namespace,
KeyOfServiceInK8s = reRoute.ServiceName,
KubeNamespace = string.IsNullOrEmpty(reRoute.ServiceNamespace) ? config.Namespace : reRoute.ServiceNamespace
};

var k8sServiceDiscoveryProvider = new Kube(k8sRegistryConfiguration, factory, kubeClient);
Expand Down
8 changes: 8 additions & 0 deletions src/Ocelot/Configuration/Builder/DownstreamReRouteBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public class DownstreamReRouteBuilder
private RateLimitOptions _rateLimitOptions;
private bool _useServiceDiscovery;
private string _serviceName;
private string _serviceNamespace;
private List<HeaderFindAndReplace> _upstreamHeaderFindAndReplace;
private List<HeaderFindAndReplace> _downstreamHeaderFindAndReplace;
private readonly List<DownstreamHostAndPort> _downstreamAddresses;
Expand Down Expand Up @@ -185,6 +186,12 @@ public DownstreamReRouteBuilder WithServiceName(string serviceName)
_serviceName = serviceName;
return this;
}

public DownstreamReRouteBuilder WithServiceNamespace(string serviceNamespace)
{
_serviceNamespace = serviceNamespace;
return this;
}

public DownstreamReRouteBuilder WithUpstreamHeaderFindAndReplace(List<HeaderFindAndReplace> upstreamHeaderFindAndReplace)
{
Expand Down Expand Up @@ -243,6 +250,7 @@ public DownstreamReRoute Build()
_downstreamHeaderFindAndReplace,
_downstreamAddresses,
_serviceName,
_serviceNamespace,
_httpHandlerOptions,
_useServiceDiscovery,
_enableRateLimiting,
Expand Down
1 change: 1 addition & 0 deletions src/Ocelot/Configuration/Creator/ReRoutesCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ private DownstreamReRoute SetUpDownstreamReRoute(FileReRoute fileReRoute, FileGl
.WithRateLimitOptions(rateLimitOption)
.WithHttpHandlerOptions(httpHandlerOptions)
.WithServiceName(fileReRoute.ServiceName)
.WithServiceNamespace(fileReRoute.ServiceNamespace)
.WithUseServiceDiscovery(fileReRouteOptions.UseServiceDiscovery)
.WithUpstreamHeaderFindAndReplace(hAndRs.Upstream)
.WithDownstreamHeaderFindAndReplace(hAndRs.Downstream)
Expand Down
3 changes: 3 additions & 0 deletions src/Ocelot/Configuration/DownstreamReRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public DownstreamReRoute(
List<HeaderFindAndReplace> downstreamHeadersFindAndReplace,
List<DownstreamHostAndPort> downstreamAddresses,
string serviceName,
string serviceNamespace,
HttpHandlerOptions httpHandlerOptions,
bool useServiceDiscovery,
bool enableEndpointEndpointRateLimiting,
Expand Down Expand Up @@ -47,6 +48,7 @@ public DownstreamReRoute(
DownstreamHeadersFindAndReplace = downstreamHeadersFindAndReplace ?? new List<HeaderFindAndReplace>();
DownstreamAddresses = downstreamAddresses ?? new List<DownstreamHostAndPort>();
ServiceName = serviceName;
ServiceNamespace = serviceNamespace;
HttpHandlerOptions = httpHandlerOptions;
UseServiceDiscovery = useServiceDiscovery;
EnableEndpointEndpointRateLimiting = enableEndpointEndpointRateLimiting;
Expand Down Expand Up @@ -76,6 +78,7 @@ public DownstreamReRoute(
public List<HeaderFindAndReplace> DownstreamHeadersFindAndReplace { get; }
public List<DownstreamHostAndPort> DownstreamAddresses { get; }
public string ServiceName { get; }
public string ServiceNamespace { get; }
public HttpHandlerOptions HttpHandlerOptions { get; }
public bool UseServiceDiscovery { get; }
public bool EnableEndpointEndpointRateLimiting { get; }
Expand Down
1 change: 1 addition & 0 deletions src/Ocelot/Configuration/File/FileReRoute.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public FileReRoute()
public FileCacheOptions FileCacheOptions { get; set; }
public bool ReRouteIsCaseSensitive { get; set; }
public string ServiceName { get; set; }
public string ServiceNamespace { get; set; }
public string DownstreamScheme { get; set; }
public FileQoSOptions QoSOptions { get; set; }
public FileLoadBalancerOptions LoadBalancerOptions { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ namespace Ocelot.ServiceDiscovery
using Providers;
using System;

public delegate IServiceDiscoveryProvider ServiceDiscoveryFinderDelegate(IServiceProvider provider, ServiceProviderConfiguration config, string key);
public delegate IServiceDiscoveryProvider ServiceDiscoveryFinderDelegate(IServiceProvider provider, ServiceProviderConfiguration config, DownstreamReRoute reRoute);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public Response<IServiceDiscoveryProvider> Get(ServiceProviderConfiguration serv
{
if (reRoute.UseServiceDiscovery)
{
return GetServiceDiscoveryProvider(serviceConfig, reRoute.ServiceName);
return GetServiceDiscoveryProvider(serviceConfig, reRoute);
}

var services = new List<Service>();
Expand All @@ -42,17 +42,17 @@ public Response<IServiceDiscoveryProvider> Get(ServiceProviderConfiguration serv
return new OkResponse<IServiceDiscoveryProvider>(new ConfigurationServiceProvider(services));
}

private Response<IServiceDiscoveryProvider> GetServiceDiscoveryProvider(ServiceProviderConfiguration config, string key)
private Response<IServiceDiscoveryProvider> GetServiceDiscoveryProvider(ServiceProviderConfiguration config, DownstreamReRoute reRoute)
{
if (config.Type?.ToLower() == "servicefabric")
{
var sfConfig = new ServiceFabricConfiguration(config.Host, config.Port, key);
var sfConfig = new ServiceFabricConfiguration(config.Host, config.Port, reRoute.ServiceName);
return new OkResponse<IServiceDiscoveryProvider>(new ServiceFabricServiceDiscoveryProvider(sfConfig));
}

if (_delegates != null)
{
var provider = _delegates?.Invoke(_provider, config, key);
var provider = _delegates?.Invoke(_provider, config, reRoute);

if (provider.GetType().Name.ToLower() == config.Type.ToLower())
{
Expand Down
17 changes: 14 additions & 3 deletions test/Ocelot.UnitTests/Consul/ProviderFactoryTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Ocelot.UnitTests.Consul
using Ocelot.Configuration.Builder;

namespace Ocelot.UnitTests.Consul
{
using Microsoft.Extensions.DependencyInjection;
using Moq;
Expand Down Expand Up @@ -29,15 +31,24 @@ public ProviderFactoryTests()
[Fact]
public void should_return_ConsulServiceDiscoveryProvider()
{
var provider = ConsulProviderFactory.Get(_provider, new ServiceProviderConfiguration("", "", 1, "", "", 1), "");
var reRoute = new DownstreamReRouteBuilder()
.WithServiceName("")
.Build();

var provider = ConsulProviderFactory.Get(_provider, new ServiceProviderConfiguration("", "", 1, "", "", 1), reRoute);
provider.ShouldBeOfType<Consul>();
}

[Fact]
public void should_return_PollingConsulServiceDiscoveryProvider()
{
var stopsPollerFromPolling = 10000;
var provider = ConsulProviderFactory.Get(_provider, new ServiceProviderConfiguration("pollconsul", "", 1, "", "", stopsPollerFromPolling), "");

var reRoute = new DownstreamReRouteBuilder()
.WithServiceName("")
.Build();

var provider = ConsulProviderFactory.Get(_provider, new ServiceProviderConfiguration("pollconsul", "", 1, "", "", stopsPollerFromPolling), reRoute);
provider.ShouldBeOfType<PollConsul>();
}
}
Expand Down
5 changes: 4 additions & 1 deletion test/Ocelot.UnitTests/Eureka/EurekaProviderFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ public void should_get()
var services = new ServiceCollection();
services.AddSingleton<IDiscoveryClient>(client.Object);
var sp = services.BuildServiceProvider();
var provider = EurekaProviderFactory.Get(sp, config, null);
var reRoute = new DownstreamReRouteBuilder()
.WithServiceName("")
.Build();
var provider = EurekaProviderFactory.Get(sp, config, reRoute);
provider.ShouldBeOfType<Eureka>();
}
}
Expand Down

0 comments on commit e1d7f28

Please sign in to comment.