From af3c39da6a85685cc458171f2d20aa75bd7b5af9 Mon Sep 17 00:00:00 2001 From: Sergey Borodachev Date: Sat, 18 Aug 2018 04:19:41 +0400 Subject: [PATCH 1/2] Addressing issue #559 (Infinite configuration file growth when ASPNETCORE_ENVIRONMENT is defined) Made IHostingEnvironment optional for AddOcelot() to avoid breaking change. --- .../ConfigurationBuilderExtensions.cs | 30 ++++--- .../ConfigurationBuilderExtensionsTests.cs | 82 +++++++++++++++++-- 2 files changed, 93 insertions(+), 19 deletions(-) diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index 7eb94329..6c6e2b48 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -9,6 +9,7 @@ namespace Ocelot.DependencyInjection using System.Text.RegularExpressions; using Configuration.File; using Newtonsoft.Json; + using Microsoft.AspNetCore.Hosting; public static class ConfigurationBuilderExtensions { @@ -28,32 +29,37 @@ namespace Ocelot.DependencyInjection return builder; } - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder) + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, IHostingEnvironment env = null) { - const string pattern = "(?i)ocelot\\.([a-zA-Z0-9]*)(\\.json)$"; + const string primaryConfigFile = "ocelot.json"; - var reg = new Regex(pattern); + const string globalConfigFile = "ocelot.global.json"; - var files = Directory.GetFiles(".") - .Where(path => reg.IsMatch(path)) + const string subConfigPattern = @"^ocelot\.[a-zA-Z0-9]+\.json$"; + + string excludeConfigName = env?.EnvironmentName != null ? $"ocelot.{env.EnvironmentName}.json" : string.Empty; + + var reg = new Regex(subConfigPattern, RegexOptions.IgnoreCase | RegexOptions.Singleline); + + var files = new DirectoryInfo(".") + .EnumerateFiles() + .Where(fi => reg.IsMatch(fi.Name) && (fi.Name != excludeConfigName)) .ToList(); var fileConfiguration = new FileConfiguration(); foreach (var file in files) { - // windows and unix sigh... - if(files.Count > 1 && (file == "./ocelot.json" || file == ".\\ocelot.json")) + if(files.Count > 1 && file.Name.Equals(primaryConfigFile, StringComparison.OrdinalIgnoreCase)) { continue; } - var lines = File.ReadAllText(file); + var lines = File.ReadAllText(file.FullName); var config = JsonConvert.DeserializeObject(lines); - // windows and unix sigh... - if (file == "./ocelot.global.json" || file == ".\\ocelot.global.json") + if (file.Name.Equals(globalConfigFile, StringComparison.OrdinalIgnoreCase)) { fileConfiguration.GlobalConfiguration = config.GlobalConfiguration; } @@ -64,9 +70,9 @@ namespace Ocelot.DependencyInjection var json = JsonConvert.SerializeObject(fileConfiguration); - File.WriteAllText("ocelot.json", json); + File.WriteAllText(primaryConfigFile, json); - builder.AddJsonFile("ocelot.json"); + builder.AddJsonFile(primaryConfigFile); return builder; } diff --git a/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs b/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs index 7c29977d..630b069c 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs @@ -9,6 +9,8 @@ using Shouldly; using TestStack.BDDfy; using Xunit; + using Moq; + using Microsoft.AspNetCore.Hosting; public class ConfigurationBuilderExtensionsTests { @@ -19,6 +21,18 @@ private FileConfiguration _reRouteA; private FileConfiguration _reRouteB; private FileConfiguration _aggregate; + private FileConfiguration _envSpecific; + + public ConfigurationBuilderExtensionsTests() + { + // Clean up config files before each test + var subConfigFiles = new DirectoryInfo(".").GetFiles("ocelot.*.json"); + + foreach(var config in subConfigFiles) + { + config.Delete(); + } + } [Fact] public void should_add_base_url_to_config() @@ -32,13 +46,23 @@ [Fact] public void should_merge_files() { - this.Given(_ => GivenMultipleConfigurationFiles()) - .When(_ => WhenIAddOcelotConfiguration()) + this.Given(_ => GivenMultipleConfigurationFiles(false)) + .When(_ => WhenIAddOcelotConfiguration(false)) .Then(_ => ThenTheConfigsAreMerged()) .BDDfy(); } - private void GivenMultipleConfigurationFiles() + [Fact] + public void should_merge_files_except_env() + { + this.Given(_ => GivenMultipleConfigurationFiles(true)) + .When(_ => WhenIAddOcelotConfiguration(true)) + .Then(_ => ThenTheConfigsAreMerged()) + .And(_ => NotContainsEnvSpecificConfig()) + .BDDfy(); + } + + private void GivenMultipleConfigurationFiles(bool addEnvSpecificConfig) { _globalConfig = new FileConfiguration { @@ -140,7 +164,7 @@ { new FileAggregateReRoute { - ReRouteKeys = new List + ReRouteKeys = new List { "KeyB", "KeyBB" @@ -149,7 +173,7 @@ }, new FileAggregateReRoute { - ReRouteKeys = new List + ReRouteKeys = new List { "KeyB", "KeyBB" @@ -159,16 +183,52 @@ } }; + _envSpecific = new FileConfiguration + { + ReRoutes = new List + { + new FileReRoute + { + DownstreamScheme = "DownstreamSchemeSpec", + DownstreamPathTemplate = "DownstreamPathTemplateSpec", + Key = "KeySpec", + UpstreamHost = "UpstreamHostSpec", + UpstreamHttpMethod = new List + { + "UpstreamHttpMethodSpec" + }, + DownstreamHostAndPorts = new List + { + new FileHostAndPort + { + Host = "HostSpec", + Port = 80 + } + } + } + } + }; + File.WriteAllText("ocelot.global.json", JsonConvert.SerializeObject(_globalConfig)); File.WriteAllText("ocelot.reRoutesA.json", JsonConvert.SerializeObject(_reRouteA)); File.WriteAllText("ocelot.reRoutesB.json", JsonConvert.SerializeObject(_reRouteB)); File.WriteAllText("ocelot.aggregates.json", JsonConvert.SerializeObject(_aggregate)); + + if (addEnvSpecificConfig) + { + File.WriteAllText("ocelot.Env.json", JsonConvert.SerializeObject(_envSpecific)); + } } - private void WhenIAddOcelotConfiguration() + private void WhenIAddOcelotConfiguration(bool addEnv) { IConfigurationBuilder builder = new ConfigurationBuilder(); - builder.AddOcelot(); + + var hostingEnvironment = new Mock(); + hostingEnvironment.SetupGet(x => x.EnvironmentName).Returns(addEnv ? "Env" : null); + + builder.AddOcelot(hostingEnvironment.Object); + _configRoot = builder.Build(); } @@ -208,6 +268,14 @@ fc.Aggregates.Count.ShouldBe(_aggregate.Aggregates.Count); } + private void NotContainsEnvSpecificConfig() + { + var fc = (FileConfiguration)_configRoot.Get(typeof(FileConfiguration)); + fc.ReRoutes.ShouldNotContain(x => x.DownstreamScheme == _envSpecific.ReRoutes[0].DownstreamScheme); + fc.ReRoutes.ShouldNotContain(x => x.DownstreamPathTemplate == _envSpecific.ReRoutes[0].DownstreamPathTemplate); + fc.ReRoutes.ShouldNotContain(x => x.Key == _envSpecific.ReRoutes[0].Key); + } + private void GivenTheBaseUrl(string baseUrl) { #pragma warning disable CS0618 From dcc3f0deae9da4c3a41a0dc68fa6881bc951100c Mon Sep 17 00:00:00 2001 From: Tom Gardham-Pallister Date: Thu, 20 Sep 2018 18:33:00 +0100 Subject: [PATCH 2/2] #559 +semver: breaking always use environment when working out AddOcelot on builder --- docs/features/configuration.rst | 18 ++++++++++++++- .../ConfigurationBuilderExtensions.cs | 4 ++-- .../ConfigurationBuilderExtensionsTests.cs | 23 ++++++++++++------- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/docs/features/configuration.rst b/docs/features/configuration.rst index 4790afc5..dfe30c5a 100644 --- a/docs/features/configuration.rst +++ b/docs/features/configuration.rst @@ -107,7 +107,7 @@ Instead of adding the configuration directly e.g. AddJsonFile("ocelot.json") you .SetBasePath(hostingContext.HostingEnvironment.ContentRootPath) .AddJsonFile("appsettings.json", true, true) .AddJsonFile($"appsettings.{hostingContext.HostingEnvironment.EnvironmentName}.json", true, true) - .AddOcelot() + .AddOcelot(hostingContext.HostingEnvironment) .AddEnvironmentVariables(); }) @@ -117,6 +117,22 @@ The way Ocelot merges the files is basically load them, loop over them, add any At the moment there is no validation at this stage it only happens when Ocelot validates the final merged configuration. This is something to be aware of when you are investigating problems. I would advise always checking what is in ocelot.json if you have any problems. +You can also give Ocelot a specific path to look in for the configuration files like below. + +.. code-block:: csharp + + .ConfigureAppConfiguration((hostingContext, config) => + { + config + .SetBasePath(hostingContext.HostingEnvironment.ContentRootPath) + .AddJsonFile("appsettings.json", true, true) + .AddJsonFile($"appsettings.{hostingContext.HostingEnvironment.EnvironmentName}.json", true, true) + .AddOcelot("/foo/bar", hostingContext.HostingEnvironment) + .AddEnvironmentVariables(); + }) + +Ocelot needs the HostingEnvironment so it know's to exclude anything environment specific from the algorithm. + Store configuration in consul ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs index af6351ea..02b5a17b 100644 --- a/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs +++ b/src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs @@ -29,12 +29,12 @@ namespace Ocelot.DependencyInjection return builder; } - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, IHostingEnvironment env = null) + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, IHostingEnvironment env) { return builder.AddOcelot(".", env); } - public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, string folder, IHostingEnvironment env = null) + public static IConfigurationBuilder AddOcelot(this IConfigurationBuilder builder, string folder, IHostingEnvironment env) { const string primaryConfigFile = "ocelot.json"; diff --git a/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs b/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs index da2bc341..465c9173 100644 --- a/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs +++ b/test/Ocelot.UnitTests/DependencyInjection/ConfigurationBuilderExtensionsTests.cs @@ -22,9 +22,12 @@ private FileConfiguration _reRouteB; private FileConfiguration _aggregate; private FileConfiguration _envSpecific; + private Mock _hostingEnvironment; + public ConfigurationBuilderExtensionsTests() { + _hostingEnvironment = new Mock(); // Clean up config files before each test var subConfigFiles = new DirectoryInfo(".").GetFiles("ocelot.*.json"); @@ -47,7 +50,8 @@ public void should_merge_files() { this.Given(_ => GivenMultipleConfigurationFiles("", false)) - .When(_ => WhenIAddOcelotConfiguration(false)) + .And(_ => GivenTheEnvironmentIs(null)) + .When(_ => WhenIAddOcelotConfiguration()) .Then(_ => ThenTheConfigsAreMerged()) .BDDfy(); } @@ -56,7 +60,8 @@ public void should_merge_files_except_env() { this.Given(_ => GivenMultipleConfigurationFiles("", true)) - .When(_ => WhenIAddOcelotConfiguration(true)) + .And(_ => GivenTheEnvironmentIs("Env")) + .When(_ => WhenIAddOcelotConfiguration()) .Then(_ => ThenTheConfigsAreMerged()) .And(_ => NotContainsEnvSpecificConfig()) .BDDfy(); @@ -241,14 +246,16 @@ } } - private void WhenIAddOcelotConfiguration(bool addEnv) + private void GivenTheEnvironmentIs(string env) + { + _hostingEnvironment.SetupGet(x => x.EnvironmentName).Returns(env); + } + + private void WhenIAddOcelotConfiguration() { IConfigurationBuilder builder = new ConfigurationBuilder(); - var hostingEnvironment = new Mock(); - hostingEnvironment.SetupGet(x => x.EnvironmentName).Returns(addEnv ? "Env" : null); - - builder.AddOcelot(hostingEnvironment.Object); + builder.AddOcelot(_hostingEnvironment.Object); _configRoot = builder.Build(); } @@ -256,7 +263,7 @@ private void WhenIAddOcelotConfigurationWithSpecificFolder(string folder) { IConfigurationBuilder builder = new ConfigurationBuilder(); - builder.AddOcelot(folder); + builder.AddOcelot(folder, _hostingEnvironment.Object); _configRoot = builder.Build(); }