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

#977 Enable validation of DI scopes in Kube and PollKube discovery providers #2180

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Ocelot.Provider.Kubernetes/Kube.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public virtual async Task<List<Service>> GetAsync()
}

private Task<EndpointsV1> GetEndpoint() => _kubeApi
.ResourceClient(client => new EndPointClientV1(client))
.ResourceClient<IEndPointClient>(client => new EndPointClientV1(client))
.GetAsync(_configuration.KeyOfServiceInK8s, _configuration.KubeNamespace);

private bool CheckErroneousState(EndpointsV1 endpoint)
Expand Down
2 changes: 1 addition & 1 deletion src/Ocelot.Provider.Kubernetes/KubeServiceCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ protected virtual ServiceHostAndPort GetServiceHostAndPort(KubeRegistryConfigura
portV1 ??= new();
portV1.Name ??= configuration.Scheme ?? string.Empty;
Logger.LogDebug(() => $"K8s service with key '{configuration.KeyOfServiceInK8s}' and address {address.Ip}; Detected port is {portV1.Name}:{portV1.Port}. Total {ports.Count} ports of [{string.Join(',', ports.Select(p => p.Name))}].");
return new ServiceHostAndPort(address.Ip, portV1.Port, portV1.Name);
return new ServiceHostAndPort(address.Ip, portV1.Port ?? 80, portV1.Name);
}

protected virtual string GetServiceId(KubeRegistryConfiguration configuration, EndpointsV1 endpoint, EndpointSubsetV1 subset, EndpointAddressV1 address)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

namespace Ocelot.Provider.Kubernetes
{
public static class KubernetesProviderFactory
public static class KubernetesProviderFactory // TODO : IServiceDiscoveryProviderFactory
{
/// <summary>
/// String constant used for provider type definition.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
<NoWarn>1591</NoWarn>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="KubeClient" Version="2.5.10" />
<PackageReference Include="KubeClient.Extensions.DependencyInjection" Version="2.5.10" />
raman-m marked this conversation as resolved.
Show resolved Hide resolved
<PackageReference Include="KubeClient" Version="2.5.12" />
<PackageReference Include="KubeClient.Extensions.DependencyInjection" Version="2.5.12" />
<PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.556">
<PrivateAssets>all</PrivateAssets>
</PackageReference>
Expand Down
23 changes: 21 additions & 2 deletions src/Ocelot.Provider.Kubernetes/OcelotBuilderExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Ocelot.DependencyInjection;
using Ocelot.Provider.Kubernetes.Interfaces;

Expand All @@ -9,10 +11,27 @@ public static class OcelotBuilderExtensions
public static IOcelotBuilder AddKubernetes(this IOcelotBuilder builder, bool usePodServiceAccount = true)
{
builder.Services
.AddKubeClient(usePodServiceAccount)
.AddSingleton(KubernetesProviderFactory.Get)

// Sweet couple
.AddSingleton<IKubeApiClient, KubeApiClient>(KubeApiClientFactory) // TODO Revert to .AddKubeClient(usePodServiceAccount) after making KubernetesProviderFactory as IServiceDiscoveryProviderFactory
.AddSingleton(KubernetesProviderFactory.Get) // TODO Must be removed after deprecation of ServiceDiscoveryFinderDelegate in favor of IServiceDiscoveryProviderFactory

.AddSingleton<IKubeServiceBuilder, KubeServiceBuilder>()
.AddSingleton<IKubeServiceCreator, KubeServiceCreator>();

return builder;

KubeApiClient KubeApiClientFactory(IServiceProvider sp)
raman-m marked this conversation as resolved.
Show resolved Hide resolved
{
if (usePodServiceAccount)
{
return KubeApiClient.CreateFromPodServiceAccount(sp.GetService<ILoggerFactory>());
}

KubeClientOptions options = sp.GetRequiredService<IOptions<KubeClientOptions>>().Value;
options.LoggerFactory ??= sp.GetService<ILoggerFactory>();

return KubeApiClient.Create(options);
}
}
}
raman-m marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,28 +15,26 @@
using Ocelot.ServiceDiscovery.Providers;
using Ocelot.Values;
using System.Runtime.CompilerServices;
using System.Text;

namespace Ocelot.AcceptanceTests.ServiceDiscovery;

public sealed class KubernetesServiceDiscoveryTests : ConcurrentSteps, IDisposable
{
private readonly string _kubernetesUrl;
private readonly IKubeApiClient _clientFactory;
private readonly ServiceHandler _kubernetesHandler;
private string _receivedToken;
private readonly Action<KubeClientOptions> _kubeClientOptionsConfigure;

public KubernetesServiceDiscoveryTests()
{
_kubernetesUrl = DownstreamUrl(PortFinder.GetRandomPort());
var option = new KubeClientOptions
_kubeClientOptionsConfigure = opts =>
{
ApiEndPoint = new Uri(_kubernetesUrl),
AccessToken = "txpc696iUhbVoudg164r93CxDTrKRVWG",
AuthStrategy = KubeAuthStrategy.BearerToken,
AllowInsecure = true,
opts.ApiEndPoint = new Uri(_kubernetesUrl);
opts.AccessToken = "txpc696iUhbVoudg164r93CxDTrKRVWG";
opts.AuthStrategy = KubeAuthStrategy.BearerToken;
opts.AllowInsecure = true;
};
_clientFactory = KubeApiClient.Create(option);
_kubernetesHandler = new();
}

Expand All @@ -62,7 +60,7 @@ public void ShouldReturnServicesFromK8s()
this.Given(x => GivenServiceInstanceIsRunning(downstreamUrl, downstreamResponse))
.And(x => x.GivenThereIsAFakeKubernetesProvider(endpoints, serviceName, namespaces))
.And(_ => GivenThereIsAConfiguration(configuration))
.And(_ => GivenOcelotIsRunningWithServices(WithKubernetes))
.And(_ => GivenOcelotIsRunningWithServices(WithKubernetes, true))
.When(_ => WhenIGetUrlOnTheApiGateway("/"))
.Then(_ => ThenTheStatusCodeShouldBe(HttpStatusCode.OK))
.And(_ => ThenTheResponseBodyShouldBe($"1:{downstreamResponse}"))
Expand Down Expand Up @@ -102,7 +100,7 @@ public void ShouldReturnServicesByPortNameAsDownstreamScheme(string downstreamSc
this.Given(x => GivenServiceInstanceIsRunning(downstreamUrl, nameof(ShouldReturnServicesByPortNameAsDownstreamScheme)))
.And(x => x.GivenThereIsAFakeKubernetesProvider(endpoints, serviceName, namespaces))
.And(_ => GivenThereIsAConfiguration(configuration))
.And(_ => GivenOcelotIsRunningWithServices(WithKubernetes))
.And(_ => GivenOcelotIsRunningWithServices(WithKubernetes, true))
.When(_ => WhenIGetUrlOnTheApiGateway("/api/example/1"))
.Then(_ => ThenTheStatusCodeShouldBe(statusCode))
.And(_ => ThenTheResponseBodyShouldBe(downstreamScheme == "http"
Expand Down Expand Up @@ -178,7 +176,7 @@ public void ShouldHighlyLoadOnUnstableKubeProvider_WithRoundRobinLoadBalancing(i
var configuration = GivenKubeConfiguration(namespaces, route);
GivenMultipleServiceInstancesAreRunning(downstreamUrls, downstreamResponses);
GivenThereIsAConfiguration(configuration);
GivenOcelotIsRunningWithServices(WithKubernetesAndRoundRobin);
GivenOcelotIsRunningWithServices(WithKubernetesAndRoundRobin, true);
return (endpoints, servicePorts);
}

Expand Down Expand Up @@ -310,16 +308,18 @@ private void GivenThereIsAFakeKubernetesProvider(EndpointsV1 endpoints, bool isS
});
}

private void WithKubernetes(IServiceCollection services) => services
.AddOcelot().AddKubernetes()
.Services.RemoveAll<IKubeApiClient>().AddSingleton(_clientFactory);
private static ServiceDescriptor GetValidateScopesDescriptor()
=> ServiceDescriptor.Singleton<IServiceProviderFactory<IServiceCollection>>(
new DefaultServiceProviderFactory(new() { ValidateScopes = true }));
private IOcelotBuilder AddKubernetes(IServiceCollection services) => services
.Configure(_kubeClientOptionsConfigure)
.Replace(GetValidateScopesDescriptor())
.AddOcelot().AddKubernetes(false);

private void WithKubernetesAndRoundRobin(IServiceCollection services) => services
.AddOcelot().AddKubernetes()
private void WithKubernetes(IServiceCollection services) => AddKubernetes(services);
private void WithKubernetesAndRoundRobin(IServiceCollection services) => AddKubernetes(services)
.AddCustomLoadBalancer<RoundRobinAnalyzer>(GetRoundRobinAnalyzer)
.Services
.RemoveAll<IKubeApiClient>().AddSingleton(_clientFactory)
.RemoveAll<IKubeServiceCreator>().AddSingleton<IKubeServiceCreator, FakeKubeServiceCreator>();
.Services.RemoveAll<IKubeServiceCreator>().AddSingleton<IKubeServiceCreator, FakeKubeServiceCreator>();

private int _k8sCounter, _k8sServiceGeneration;
private static readonly object K8sCounterLocker = new();
Expand All @@ -343,7 +343,7 @@ protected override ServiceHostAndPort GetServiceHostAndPort(KubeRegistryConfigur
var index = subset.Addresses.IndexOf(address);
var portV1 = ports[index];
Logger.LogDebug(() => $"K8s service with key '{configuration.KeyOfServiceInK8s}' and address {address.Ip}; Detected port is {portV1.Name}:{portV1.Port}. Total {ports.Count} ports of [{string.Join(',', ports.Select(p => p.Name))}].");
return new ServiceHostAndPort(address.Ip, portV1.Port, portV1.Name);
return new ServiceHostAndPort(address.Ip, (int)portV1.Port, portV1.Name);
}

protected override IEnumerable<string> GetServiceTags(KubeRegistryConfiguration configuration, EndpointsV1 endpoint, EndpointSubsetV1 subset, EndpointAddressV1 address)
Expand Down
3 changes: 3 additions & 0 deletions test/Ocelot.UnitTests/Kubernetes/KubeServiceCreatorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ public void Create_NullArgs_ReturnedEmpty(bool isConfiguration, bool isEndpoint,
Assert.Empty(actual);
}

[Trait("Bug", "977")]
[Fact(DisplayName = "Create: With empty args -> No exceptions during creation")]
public void Create_NotNullButEmptyArgs_CreatedEmptyService()
{
Expand All @@ -94,6 +95,8 @@ public void Create_NotNullButEmptyArgs_CreatedEmptyService()
var actualService = actual.SingleOrDefault();
Assert.NotNull(actualService);
Assert.Null(actualService.Name);
Assert.NotNull(actualService.HostAndPort);
Assert.Equal(80, actualService.HostAndPort.DownstreamPort);
}

[Fact]
Expand Down
6 changes: 5 additions & 1 deletion test/Ocelot.UnitTests/Kubernetes/KubeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@

namespace Ocelot.UnitTests.Kubernetes;

/// <summary>
/// Contains integration tests.
/// Move to integration testing, and add at least one "happy path" unit test.
/// </summary>
public class KubeTests
{
private readonly Mock<IOcelotLoggerFactory> _factory;
Expand Down Expand Up @@ -50,7 +54,7 @@ public async Task Should_return_service_from_k8s()
receivedToken.Value.ShouldBe($"Bearer {nameof(Should_return_service_from_k8s)}");
}

[Fact]
[Fact] // This is not unit test! LoL :) This should be an integration test or even an acceptance test...
[Trait("Bug", "2110")]
public async Task Should_return_single_service_from_k8s_during_concurrent_calls()
{
Expand Down
88 changes: 88 additions & 0 deletions test/Ocelot.UnitTests/Kubernetes/KubernetesProviderFactoryTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
using KubeClient;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Ocelot.Configuration;
using Ocelot.Configuration.Builder;
using Ocelot.DependencyInjection;
using Ocelot.Provider.Kubernetes;
using Ocelot.Provider.Kubernetes.Interfaces;
using Ocelot.ServiceDiscovery;
using Ocelot.ServiceDiscovery.Providers;
using System.Runtime.CompilerServices;

namespace Ocelot.UnitTests.Kubernetes;

public class KubernetesProviderFactoryTests : UnitTest
{
private readonly IOcelotBuilder _builder;

public KubernetesProviderFactoryTests()
{
_builder = new ServiceCollection()
.AddOcelot().AddKubernetes();
}

[Theory]
[Trait("Bug", "977")]
[InlineData(typeof(Kube))]
[InlineData(typeof(PollKube))]
public void CreateProvider_IKubeApiClientHasOriginalLifetimeWithEnabledScopesValidation_ShouldResolveProvider(Type providerType)
{
// Arrange
var kubeClient = new Mock<IKubeApiClient>();
kubeClient.Setup(x => x.ResourceClient(It.IsAny<Func<IKubeApiClient, IEndPointClient>>()))
.Returns(Mock.Of<IEndPointClient>());
var descriptor = _builder.Services.First(x => x.ServiceType == typeof(IKubeApiClient));
_builder.Services.Replace(ServiceDescriptor.Describe(descriptor.ServiceType, _ => kubeClient.Object, descriptor.Lifetime));
var serviceProvider = _builder.Services.BuildServiceProvider(validateScopes: true);
var config = GivenServiceProvider(providerType.Name);
var route = GivenRoute();

// Act
IServiceDiscoveryProvider actual = null;
var resolving = () => actual = serviceProvider
.GetRequiredService<ServiceDiscoveryFinderDelegate>() // returns KubernetesProviderFactory.Get instance
.Invoke(serviceProvider, config, route);

// Assert
resolving.ShouldNotThrow();
actual.ShouldNotBeNull().ShouldBeOfType(providerType);
}

[Theory]
[Trait("Bug", "977")]
[InlineData(nameof(Kube))]
[InlineData(nameof(PollKube))]
public void CreateProvider_IKubeApiClientHasScopedLifetimeWithEnabledScopesValidation_ShouldFailToResolve(string providerType)
{
// Arrange
var descriptor = ServiceDescriptor.Describe(typeof(IKubeApiClient), _ => Mock.Of<IKubeApiClient>(), ServiceLifetime.Scoped);
_builder.Services.Replace(descriptor);
var serviceProvider = _builder.Services.BuildServiceProvider(validateScopes: true);
var config = GivenServiceProvider(providerType);
var route = GivenRoute();

// Act
IServiceDiscoveryProvider actual = null;
var resolving = () => actual = serviceProvider
.GetRequiredService<ServiceDiscoveryFinderDelegate>() // returns KubernetesProviderFactory.Get instance
.Invoke(serviceProvider, config, route);

// Assert
var ex = resolving.ShouldThrow<InvalidOperationException>();
ex.Message.ShouldContain("Cannot resolve scoped service 'KubeClient.IKubeApiClient' from root provider");
actual.ShouldBeNull();
}

private static ServiceProviderConfiguration GivenServiceProvider(string type) => new(
type: type,
scheme: string.Empty,
host: string.Empty,
port: 1,
token: string.Empty,
configurationKey: string.Empty,
pollingInterval: 1);
raman-m marked this conversation as resolved.
Show resolved Hide resolved

private static DownstreamRoute GivenRoute([CallerMemberName] string serviceName = "test-service")
=> new DownstreamRouteBuilder().WithServiceName(serviceName).Build();
}
Loading