ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.4k stars 1.64k forks source link

Infinite configuration file growth when ASPNETCORE_ENVIRONMENT is defined #559

Closed snb83 closed 6 years ago

snb83 commented 6 years ago

Expected Behavior

Ocelot should be able to read and merge multiple configuration files (ocelot.*.json).

Symptom

Merged configuration files are growing in size (in an arithmetic progression) on each API Gateway restart. 2nd and all subsequent API Gateway app runs have broken configuration.

Problem

When

_ASPNETCOREENVIRONMENT environment variable is defined

and Ocelot has at least one dedicated configuration file initially deployed (Ocelot.Something.json)

then

On first run Ocelot produces "Ocelot.{env.EnvironmentName}.json" file containing initial content of "Ocelot.Something.json".

On second run Ocelot uses "Ocelot.{env.EnvironmentName}.json" and "Ocelot.Something.json" as input for config merge operation and produces new "Ocelot.{env.EnvironmentName}.json" file that contains concatenation of existing "Ocelot.{env.EnvironmentName}.json" and "Ocelot.Something.json". The resulting file gets "Ocelot.Something.json" content duplicated two times (since that moment API Gateway configuration is broken).

On third run ... Ocelot does the same operation (again). The merged configuration file becomes 3x of "Ocelot.Something.json"

The problem seems to be caused by interference of business-logic of two classes: Configuration\Repository\DiskFileConfigurationRepository.cs and DependencyInjection\ConfigurationBuilderExtensions.cs

In DiskFileConfigurationRepository.cs file the configuration file name is built from current IHostingEnvironment.EnvironmentName value:

public DiskFileConfigurationRepository(IHostingEnvironment hostingEnvironment)
{
     _configFilePath = $"{AppContext.BaseDirectory}/{ConfigurationFileName}{(string.IsNullOrEmpty(hostingEnvironment.EnvironmentName) ? string.Empty : ".")}{hostingEnvironment.EnvironmentName}.json";
}

then FileConfiguration is saved like so:

public Task<Response> Set(FileConfiguration fileConfiguration)
{
    string jsonConfiguration = JsonConvert.SerializeObject(fileConfiguration, Formatting.Indented);

    lock(_lock)
    {
        if (System.IO.File.Exists(_configFilePath))
        {
            System.IO.File.Delete(_configFilePath);
        }

        System.IO.File.WriteAllText(_configFilePath, jsonConfiguration);
    }

    return Task.FromResult<Response>(new OkResponse());
}

ConfigurationBuilderExtensions.cs contains AddOcelot() extension method which knows nothing about IHostingEnvironment and assumes that any file matching pattern "(?i)ocelot\.([a-zA-Z0-9]*)(\.json)$" is perfectly valid candidate for merge (including the file created by DiskFileConfigurationRepository).

Solution

The quick fix I can see here is to to pass IHostingEnvironment to AddOcelot() extension method to let it filter out ocelot.{ASPNETCORE_ENVIRONMENT}.json during the merge cycle.

Specifications

Comments

This issue actually breaks Continuous Integration / Continuous Deployment cycle when micro-services ecosystem is being developed and tested on IIS instances.

At the project I'm working on we have multiple .NET Core micro-services connected by API Gateway (which uses Ocelot). Each micro-service comes with it's own Gateway configuration (re-routes and aggregates). Some deployment environments have unique capabilities so we have as many ASPNETCORE_ENVIRONMENT variations as the number of unique environments.

When a micro-service is being deployed, the deployment agent replaces corresponding Ocelot.ServiceName.json file at the API Gateway root by the latest gateway configuration file from microservice bundle and restarts the API Gateway. The problem is that this only works once :) Currently our script explicitly deletes ocelot.{ASPNETCORE_ENVIRONMENT}.json before starting back the API Gateway service... but if API Gateway is being shutdown and restarted by IIS (by any reason) it breaks the Gateway service.

TomPallister commented 6 years ago

@snb83 thanks for this, I really appreciate it.

I will review ASAP.

TomPallister commented 6 years ago

fixed in 12.0.0, just released to NuGet