From 6f4db9a6e91d665d375e0c600fe2ca881e699b7d Mon Sep 17 00:00:00 2001 From: nulltoken Date: Thu, 19 May 2022 17:24:49 +0200 Subject: [PATCH] Implement ProxyConfig and ProxyConfigProvider callbacks Fix #1619 --- .../Configuration/IProxyConfigNotifier.cs | 23 +++ .../IProxyConfigProviderNotifier.cs | 23 +++ .../Management/ProxyConfigManager.cs | 46 ++++++ .../Common/InMemoryConfigProvider.cs | 117 ++++++++++++--- .../Management/ProxyConfigManagerTests.cs | 137 ++++++++++++++++++ 5 files changed, 329 insertions(+), 17 deletions(-) create mode 100644 src/ReverseProxy/Configuration/IProxyConfigNotifier.cs create mode 100644 src/ReverseProxy/Configuration/IProxyConfigProviderNotifier.cs diff --git a/src/ReverseProxy/Configuration/IProxyConfigNotifier.cs b/src/ReverseProxy/Configuration/IProxyConfigNotifier.cs new file mode 100644 index 0000000000..64f75d0065 --- /dev/null +++ b/src/ReverseProxy/Configuration/IProxyConfigNotifier.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; + +namespace Yarp.ReverseProxy.Configuration; + +/// +/// Represents an optional configuration capability. When decorating a type, +/// will allow config instances to be notified upon config applying. +/// +public interface IProxyConfigNotifier +{ + /// + /// A callback that will be triggered once changes to the configuration have been successfully applied. + /// + void SuccessfulConfigChangeApplyingCallback(); + + /// + /// A callback that will be triggered once changes to the configuration have been tried to be applied but eventually failed. + /// + void ErroredConfigChangeApplyingCallback(Exception ex); +} diff --git a/src/ReverseProxy/Configuration/IProxyConfigProviderNotifier.cs b/src/ReverseProxy/Configuration/IProxyConfigProviderNotifier.cs new file mode 100644 index 0000000000..7c2975ae83 --- /dev/null +++ b/src/ReverseProxy/Configuration/IProxyConfigProviderNotifier.cs @@ -0,0 +1,23 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; + +namespace Yarp.ReverseProxy.Configuration; + +/// +/// Represents an optional configuration provider capability. When decorating a type, +/// will allow config provider instances to be notified upon config loading. +/// +public interface IProxyConfigProviderNotifier +{ + /// + /// A callback that will be triggered once the configuration have been successfully loaded. + /// + void SuccessfulConfigLoadingCallback(); + + /// + /// A callback that will be triggered once the configuration have been tried to be loaded but eventually failed. + /// + void ErroredConfigLoadingCallback(Exception ex); +} diff --git a/src/ReverseProxy/Management/ProxyConfigManager.cs b/src/ReverseProxy/Management/ProxyConfigManager.cs index 3e23338d3c..099bbc91e8 100644 --- a/src/ReverseProxy/Management/ProxyConfigManager.cs +++ b/src/ReverseProxy/Management/ProxyConfigManager.cs @@ -158,10 +158,23 @@ internal async Task InitialLoadAsync() _configs[i] = new ConfigState(provider, config); routes.AddRange(config.Routes ?? Array.Empty()); clusters.AddRange(config.Clusters ?? Array.Empty()); + + if (provider is IProxyConfigProviderNotifier) + { + ((IProxyConfigProviderNotifier)provider).SuccessfulConfigLoadingCallback(); + } } await ApplyConfigAsync(routes, clusters); + foreach (var c in _configs) + { + if (c.LatestConfig is IProxyConfigNotifier) + { + ((IProxyConfigNotifier)c.LatestConfig).SuccessfulConfigChangeApplyingCallback(); + } + } + ListenForConfigChanges(); } catch (Exception ex) @@ -182,6 +195,9 @@ private async Task ReloadConfigAsync() var sourcesChanged = false; var routes = new List(); var clusters = new List(); + + var notifiers = new List(); + foreach (var instance in _configs) { try @@ -193,12 +209,30 @@ private async Task ReloadConfigAsync() instance.LatestConfig = config; instance.LoadFailed = false; sourcesChanged = true; + + if (instance.Provider is IProxyConfigProviderNotifier) + { + // When callback is defined, notify the configuration provider of the successful loading of the configuration. + ((IProxyConfigProviderNotifier)instance.Provider).SuccessfulConfigLoadingCallback(); + } + } + + if (instance.LatestConfig is IProxyConfigNotifier) + { + // And register potentially existing config notifiers for later invocation + notifiers.Add(((IProxyConfigNotifier)instance.LatestConfig)); } } catch (Exception ex) { instance.LoadFailed = true; Log.ErrorReloadingConfig(_logger, ex); + + if (instance.Provider is IProxyConfigProviderNotifier) + { + // When callback is defined, notify the configuration provider of the unsuccessful loading of the configuration. + ((IProxyConfigProviderNotifier)instance.Provider).ErroredConfigLoadingCallback(ex); + } } // If we didn't/couldn't get a new config then re-use the last one. @@ -221,10 +255,22 @@ private async Task ReloadConfigAsync() CreateEndpoints(); } } + + foreach (var notifier in notifiers) + { + // Notify all registered config callbacks of successful applying. + notifier.SuccessfulConfigChangeApplyingCallback(); + } } catch (Exception ex) { Log.ErrorApplyingConfig(_logger, ex); + + foreach (var notifier in notifiers) + { + // Notify all registered config callbacks of unsuccessful applying. + notifier.ErroredConfigChangeApplyingCallback(ex); + } } } diff --git a/test/ReverseProxy.Tests/Common/InMemoryConfigProvider.cs b/test/ReverseProxy.Tests/Common/InMemoryConfigProvider.cs index f535fddddb..95f806d60a 100644 --- a/test/ReverseProxy.Tests/Common/InMemoryConfigProvider.cs +++ b/test/ReverseProxy.Tests/Common/InMemoryConfigProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Collections.Generic; using System.Threading; using Microsoft.Extensions.Primitives; @@ -25,41 +26,123 @@ public class InMemoryConfigProvider : IProxyConfigProvider { private volatile InMemoryConfig _config; - public InMemoryConfigProvider(IReadOnlyList routes, IReadOnlyList clusters) + public InMemoryConfigProvider(IReadOnlyList routes, IReadOnlyList clusters) : this(new InMemoryConfig(routes, clusters)) + { } + + public InMemoryConfigProvider(InMemoryConfig config) { - _config = new InMemoryConfig(routes, clusters); + _config = config; } - public IProxyConfig GetConfig() => _config; + public virtual IProxyConfig GetConfig() => _config; public void Update(IReadOnlyList routes, IReadOnlyList clusters) + { + Update(new InMemoryConfig(routes, clusters)); + } + + public void Update(InMemoryConfig config) { var oldConfig = _config; - _config = new InMemoryConfig(routes, clusters); + _config = config; oldConfig.SignalChange(); } + } - private class InMemoryConfig : IProxyConfig + public class InMemoryConfig : IProxyConfig + { + private readonly CancellationTokenSource _cts = new CancellationTokenSource(); + + public InMemoryConfig(IReadOnlyList routes, IReadOnlyList clusters) { - private readonly CancellationTokenSource _cts = new CancellationTokenSource(); + Routes = routes; + Clusters = clusters; + ChangeToken = new CancellationChangeToken(_cts.Token); + } - public InMemoryConfig(IReadOnlyList routes, IReadOnlyList clusters) - { - Routes = routes; - Clusters = clusters; - ChangeToken = new CancellationChangeToken(_cts.Token); - } + public IReadOnlyList Routes { get; } - public IReadOnlyList Routes { get; } + public IReadOnlyList Clusters { get; } - public IReadOnlyList Clusters { get; } + public IChangeToken ChangeToken { get; } - public IChangeToken ChangeToken { get; } + internal void SignalChange() + { + _cts.Cancel(); + } + } + + public class InMemoryConfigProviderNotifier : InMemoryConfigProvider, IProxyConfigProviderNotifier + { + private readonly Action _successLoadingCallback; + private readonly Action _erroredLoadingCallback; + + public InMemoryConfigProviderNotifier( + InMemoryConfig config, + Action successLoadingCallback, + Action erroredLoadingCallback) : base(config) + { + _successLoadingCallback = successLoadingCallback; + _erroredLoadingCallback = erroredLoadingCallback; + } + + public InMemoryConfigProviderNotifier( + IReadOnlyList routes, + IReadOnlyList clusters, + Action successLoadingCallback, + Action erroredLoadingCallback) : base(routes, clusters) + { + _successLoadingCallback = successLoadingCallback; + _erroredLoadingCallback = erroredLoadingCallback; + } - internal void SignalChange() + public override IProxyConfig GetConfig() + { + if (ShouldConfigLoadingFail) { - _cts.Cancel(); + return null; } + + return base.GetConfig(); + } + + public bool ShouldConfigLoadingFail { get; set; } + + public void ErroredConfigLoadingCallback(Exception ex) + { + _erroredLoadingCallback(ex); + } + + public void SuccessfulConfigLoadingCallback() + { + _successLoadingCallback(); + } + } + + public class InMemoryConfigNotifier : InMemoryConfig, IProxyConfigNotifier + { + private readonly CancellationTokenSource _cts = new CancellationTokenSource(); + private readonly Action _successChangeCallback; + private readonly Action _erroredChangeCallback; + + public InMemoryConfigNotifier( + IReadOnlyList routes, + IReadOnlyList clusters, + Action successChangeCallback, + Action erroredChangeCallback) : base(routes, clusters) + { + _successChangeCallback = successChangeCallback; + _erroredChangeCallback = erroredChangeCallback; + } + + public void ErroredConfigChangeApplyingCallback(Exception ex) + { + _erroredChangeCallback(ex); + } + + public void SuccessfulConfigChangeApplyingCallback() + { + _successChangeCallback(); } } } diff --git a/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs b/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs index ec5b693657..898ea15f57 100644 --- a/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs +++ b/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs @@ -367,6 +367,143 @@ public async Task BuildConfig_TwoOverlappingConfigs_Works() Assert.Equal(TestAddress, destination.Model.Config.Address); } + [Fact] + public async Task BuildConfig_CanBeNotifiedOfProxyConfigSuccessfullAndErroredLoading() + { + bool? hasConfig1Loaded = null; + bool? hasConfig2Loaded = null; + + var configProvider1 = new InMemoryConfigProviderNotifier(new List() { }, new List() { }, () => { hasConfig1Loaded = true; }, (_) => { hasConfig1Loaded = false; }); + var configProvider2 = new InMemoryConfigProviderNotifier(new List() { }, new List() { }, () => { hasConfig2Loaded = true; }, (_) => { hasConfig2Loaded = false; }); + + var services = CreateServices(new[] { configProvider1, configProvider2 }); + + var manager = services.GetRequiredService(); + await manager.InitialLoadAsync(); + + Assert.True(hasConfig1Loaded); + Assert.True(hasConfig2Loaded); + + const string TestAddress = "https://localhost:123/"; + + var cluster1 = new ClusterConfig + { + ClusterId = "cluster1", + Destinations = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "d1", new DestinationConfig { Address = TestAddress } } + } + }; + var cluster2 = new ClusterConfig + { + ClusterId = "cluster2", + Destinations = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "d2", new DestinationConfig { Address = TestAddress } } + } + }; + + var route1 = new RouteConfig + { + RouteId = "route1", + ClusterId = "cluster1", + Match = new RouteMatch { Path = "/" } + }; + var route2 = new RouteConfig + { + RouteId = "route2", + ClusterId = "cluster2", + Match = new RouteMatch { Path = "/" } + }; + + hasConfig1Loaded = null; + hasConfig2Loaded = null; + + configProvider1.Update(new List() { route1 }, new List() { cluster1 }); + + Assert.True(hasConfig1Loaded); + Assert.Null(hasConfig2Loaded); + + hasConfig1Loaded = null; + + configProvider2.ShouldConfigLoadingFail = true; + configProvider2.Update(new List() { route2 }, new List() { cluster2 }); + + Assert.Null(hasConfig1Loaded); + Assert.False(hasConfig2Loaded); + } + + [Fact] + public async Task BuildConfig_CanBeNotifiedOfProxyConfigSuccessfullAndErroredUpdating() + { + bool? hasConfig1Updated = null; + bool? hasConfig2Updated = null; + + var config1 = new InMemoryConfigNotifier(new List() { }, new List() { }, () => { hasConfig1Updated = true; }, (_) => { hasConfig1Updated = false; }); + var config2 = new InMemoryConfigNotifier(new List() { }, new List() { }, () => { hasConfig2Updated = true; }, (_) => { hasConfig2Updated = false; }); + + var configProvider1 = new InMemoryConfigProvider(config1); + var configProvider2 = new InMemoryConfigProvider(config2); + + var services = CreateServices(new[] { configProvider1, configProvider2 }); + + var manager = services.GetRequiredService(); + var dataSource = await manager.InitialLoadAsync(); + + Assert.True(hasConfig1Updated); + Assert.True(hasConfig2Updated); + + const string TestAddress = "https://localhost:123/"; + + var cluster1 = new ClusterConfig + { + ClusterId = "cluster1", + Destinations = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "d1", new DestinationConfig { Address = TestAddress } } + } + }; + var cluster2 = new ClusterConfig + { + ClusterId = "cluster2", + Destinations = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "d2", new DestinationConfig { Address = TestAddress } } + } + }; + + var route1 = new RouteConfig + { + RouteId = "route1", + ClusterId = "cluster1", + Match = new RouteMatch { Path = "/" } + }; + var route2 = new RouteConfig + { + RouteId = "route2", + ClusterId = "cluster2", + // Missing Match here will be caught by the analysis + }; + + hasConfig1Updated = null; + hasConfig2Updated = null; + + var config1b = new InMemoryConfigNotifier(new List() { route1 }, new List() { cluster1 }, () => { hasConfig1Updated = true; }, (_) => { hasConfig1Updated = false; }); + configProvider1.Update(config1b); + + Assert.True(hasConfig1Updated); + Assert.True(hasConfig2Updated); + + hasConfig1Updated = null; + hasConfig2Updated = null; + + var config2b = new InMemoryConfigNotifier(new List() { route2 }, new List() { cluster2 }, () => { hasConfig2Updated = true; }, (_) => { hasConfig2Updated = false; }); + configProvider2.Update(config2b); + + Assert.False(hasConfig1Updated); + Assert.False(hasConfig2Updated); + } + [Fact] public async Task InitialLoadAsync_ProxyHttpClientOptionsSet_CreateAndSetHttpClient() {