From 9a6f7728542f547c354d169570e9895bcb0a7a44 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 ++++++ .../Management/ProxyConfigManagerTests.cs | 137 ++++++++++++++++++ 4 files changed, 229 insertions(+) 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/Management/ProxyConfigManagerTests.cs b/test/ReverseProxy.Tests/Management/ProxyConfigManagerTests.cs index 2a6cab209f..bff27ba63d 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() {