From a1b73af4b4a57943c081691147f3e6454f652718 Mon Sep 17 00:00:00 2001 From: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com> Date: Wed, 30 Oct 2024 02:52:54 -0700 Subject: [PATCH] Hot Reload Validation (#2405) ## Why make this change? This change fixes issue #2396 ## What is this change? In order to validate the hot-reload, first, new logic was added to the `RuntimeConfigLoader` and `FileSystemRuntimeConfigLoader`, so when hot reload occurs, it updates the new states added to the loader which are isNewConfigDetected, isNewConfigValidated, and lastValidRuntimeConfig. Once the states are updated, it will try to parse and validate the new RuntimeConfig, in the case that it fails at any step of the way, the hot-reloading process will be canceled and the RuntimeConfig file will be changed so that it uses the information from lastValidRuntimeConfig so that the user can still use their last configuration without any problems. ## How was this tested? - [X] Integration Tests - [ ] Unit Tests --------- Co-authored-by: aaron burtle Co-authored-by: Ruben Cerna Co-authored-by: Aniruddh Munde --- src/Config/ConfigFileWatcher.cs | 9 + src/Config/FileSystemRuntimeConfigLoader.cs | 22 ++- src/Config/RuntimeConfigLoader.cs | 44 ++++- .../Configurations/RuntimeConfigProvider.cs | 42 +++++ .../Configuration/ConfigurationTests.cs | 166 ++++++++++++++++++ src/Service.Tests/TestHelper.cs | 41 +++++ .../Azure.DataApiBuilder.Service.csproj | 10 ++ 7 files changed, 330 insertions(+), 4 deletions(-) diff --git a/src/Config/ConfigFileWatcher.cs b/src/Config/ConfigFileWatcher.cs index dd7d874b6c..b9384b2df4 100644 --- a/src/Config/ConfigFileWatcher.cs +++ b/src/Config/ConfigFileWatcher.cs @@ -95,6 +95,15 @@ private void OnConfigFileChange(object sender, FileSystemEventArgs e) } } } + catch (AggregateException ex) + { + // Need to remove the dependencies in startup on the RuntimeConfigProvider + // before we can have an ILogger here. + foreach (Exception exception in ex.InnerExceptions) + { + Console.WriteLine("Unable to hot reload configuration file due to " + exception.Message); + } + } catch (Exception ex) { // Need to remove the dependencies in startup on the RuntimeConfigProvider diff --git a/src/Config/FileSystemRuntimeConfigLoader.cs b/src/Config/FileSystemRuntimeConfigLoader.cs index a2a7956abe..88ab500edf 100644 --- a/src/Config/FileSystemRuntimeConfigLoader.cs +++ b/src/Config/FileSystemRuntimeConfigLoader.cs @@ -202,9 +202,20 @@ public bool TryLoadConfig( } config = RuntimeConfig; + + if (LastValidRuntimeConfig is null) + { + LastValidRuntimeConfig = RuntimeConfig; + } + return true; } + if (LastValidRuntimeConfig is not null) + { + RuntimeConfig = LastValidRuntimeConfig; + } + config = null; return false; } @@ -242,7 +253,16 @@ public override bool TryLoadKnownConfig([NotNullWhen(true)] out RuntimeConfig? c private void HotReloadConfig(ILogger? logger = null) { logger?.LogInformation(message: "Starting hot-reload process for config: {ConfigFilePath}", ConfigFilePath); - TryLoadConfig(ConfigFilePath, out _, replaceEnvVar: true); + if (!TryLoadConfig(ConfigFilePath, out _, replaceEnvVar: true)) + { + throw new DataApiBuilderException( + message: "Deserialization of the configuration file failed.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + + IsNewConfigDetected = true; + IsNewConfigValidated = false; SignalConfigChanged(); } diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index 94909157a7..eb32df1765 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -32,6 +32,12 @@ public abstract class RuntimeConfigLoader // state in place of using out params. public RuntimeConfig? RuntimeConfig; + public RuntimeConfig? LastValidRuntimeConfig; + + public bool IsNewConfigDetected; + + public bool IsNewConfigValidated; + public RuntimeConfigLoader(HotReloadEventHandler? handler = null, string? connectionString = null) { _changeToken = new DabChangeToken(); @@ -80,14 +86,14 @@ protected virtual void OnConfigChangedEvent(HotReloadEventArgs args) /// protected void SignalConfigChanged(string message = "") { + // Signal that a change has occurred to all change token listeners. + RaiseChanged(); + OnConfigChangedEvent(new HotReloadEventArgs(QUERY_MANAGER_FACTORY_ON_CONFIG_CHANGED, message)); OnConfigChangedEvent(new HotReloadEventArgs(METADATA_PROVIDER_FACTORY_ON_CONFIG_CHANGED, message)); OnConfigChangedEvent(new HotReloadEventArgs(QUERY_ENGINE_FACTORY_ON_CONFIG_CHANGED, message)); OnConfigChangedEvent(new HotReloadEventArgs(MUTATION_ENGINE_FACTORY_ON_CONFIG_CHANGED, message)); OnConfigChangedEvent(new HotReloadEventArgs(DOCUMENTOR_ON_CONFIG_CHANGED, message)); - - // Signal that a change has occurred to all change token listeners. - RaiseChanged(); } /// @@ -339,4 +345,36 @@ internal static string GetPgSqlConnectionStringWithApplicationName(string connec // Return the updated connection string. return connectionStringBuilder.ConnectionString; } + + public bool DoesConfigNeedValidation() + { + if (IsNewConfigDetected && !IsNewConfigValidated) + { + IsNewConfigDetected = false; + return true; + } + + return false; + } + + /// + /// Once the validation of the new config file is confirmed to have passed, + /// this function will save the newly resolved RuntimeConfig as the new last known good, + /// in order to have config file DAB can go into in case hot reload fails. + /// + public void SetLkgConfig() + { + IsNewConfigValidated = false; + LastValidRuntimeConfig = RuntimeConfig; + } + + /// + /// Changes the state of the config file into the last known good iteration, + /// in order to allow users to still be able to make changes in DAB even if + /// a hot reload fails. + /// + public void RestoreLkgConfig() + { + RuntimeConfig = LastValidRuntimeConfig; + } } diff --git a/src/Core/Configurations/RuntimeConfigProvider.cs b/src/Core/Configurations/RuntimeConfigProvider.cs index e58ac56f20..5e4f85124f 100644 --- a/src/Core/Configurations/RuntimeConfigProvider.cs +++ b/src/Core/Configurations/RuntimeConfigProvider.cs @@ -3,12 +3,14 @@ using System.Data.Common; using System.Diagnostics.CodeAnalysis; +using System.IO.Abstractions; using System.Net; using Azure.DataApiBuilder.Config; using Azure.DataApiBuilder.Config.Converters; using Azure.DataApiBuilder.Config.NamingPolicies; using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Service.Exceptions; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; namespace Azure.DataApiBuilder.Core.Configurations; @@ -63,6 +65,11 @@ public RuntimeConfigProvider(RuntimeConfigLoader runtimeConfigLoader) /// private void RaiseChanged() { + //First use of GetConfig during hot reload, in order to do validation of + //config file before any changes are made for hot reload. + //In case validation fails, an exception will be thrown and hot reload will be canceled. + ValidateConfig(); + DabChangeToken previousToken = Interlocked.Exchange(ref _changeToken, new DabChangeToken()); previousToken.SignalChange(); } @@ -290,6 +297,41 @@ public bool IsConfigHotReloadable() return !IsLateConfigured || !(_configLoader.RuntimeConfig?.Runtime?.Host?.Mode == HostMode.Production); } + /// + /// This function checks if there is a new config that needs to be validated + /// and validates the configuration file as well as the schema file, in the + /// case that it is not able to validate both then it will return an error. + /// + /// + public void ValidateConfig() + { + // Only used in hot reload to validate the configuration file + if (_configLoader.DoesConfigNeedValidation()) + { + IFileSystem fileSystem = new FileSystem(); + ILoggerFactory loggerFactory = new LoggerFactory(); + ILogger logger = loggerFactory.CreateLogger(); + RuntimeConfigValidator runtimeConfigValidator = new(this, fileSystem, logger, true); + + _configLoader.IsNewConfigValidated = runtimeConfigValidator.TryValidateConfig(ConfigFilePath, loggerFactory).Result; + + // Saves the lastValidRuntimeConfig as the new RuntimeConfig if it is validated for hot reload + if (_configLoader.IsNewConfigValidated) + { + _configLoader.SetLkgConfig(); + } + else + { + _configLoader.RestoreLkgConfig(); + + throw new DataApiBuilderException( + message: "Failed validation of configuration file.", + statusCode: HttpStatusCode.ServiceUnavailable, + subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization); + } + } + } + private async Task InvokeConfigLoadedHandlersAsync() { List> configLoadedTasks = new(); diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 12761850e4..c1c7041976 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -4144,6 +4144,125 @@ public async Task TestNoDepthLimitOnGrahQLInNonHostedMode(int? depthLimit) } } + /// + /// Creates a hot reload scenario in which the schema file is invalid which causes + /// hot reload to fail, then we check that the program is still able to work + /// properly by validating that the DAB engine is still using the same configuration file + /// from before the hot reload. + /// + [TestMethod] + [TestCategory(TestCategory.MSSQL)] + public void HotReloadValidationFail() + { + // Arrange + string schemaName = "testSchema.json"; + string configName = "hotreload-config.json"; + if (File.Exists(configName)) + { + File.Delete(configName); + } + + if (File.Exists(schemaName)) + { + File.Delete(schemaName); + } + + bool initialRestEnabled = true; + bool updatedRestEnabled = false; + + bool initialGQLEnabled = true; + bool updatedGQLEnabled = false; + + DataSource dataSource = new(DatabaseType.MSSQL, + ConfigurationTests.GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL), Options: null); + + RuntimeConfig initialConfig = InitRuntimeConfigForHotReload(schemaName, dataSource, initialRestEnabled, initialGQLEnabled); + + RuntimeConfig updatedConfig = InitRuntimeConfigForHotReload(schemaName, dataSource, updatedRestEnabled, updatedGQLEnabled); + + string schemaConfig = TestHelper.GenerateInvalidSchema(); + + // Not using mocked filesystem so we pick up real file changes for hot reload + FileSystem fileSystem = new(); + RuntimeConfigProvider configProvider = GenerateConfigFileAndConfigProvider(fileSystem, configName, initialConfig); + RuntimeConfig lkgRuntimeConfig = configProvider.GetConfig(); + + Assert.IsNotNull(lkgRuntimeConfig); + + // Act + // Simulate an invalid change to the schema file while the config is updated to a valid state + fileSystem.File.WriteAllText(schemaName, schemaConfig); + fileSystem.File.WriteAllText(configName, updatedConfig.ToJson()); + + // Give ConfigFileWatcher enough time to hot reload the change + System.Threading.Thread.Sleep(6000); + + RuntimeConfig newRuntimeConfig = configProvider.GetConfig(); + Assert.AreEqual(expected: lkgRuntimeConfig, actual: newRuntimeConfig); + + if (File.Exists(configName)) + { + File.Delete(configName); + } + + if (File.Exists(schemaName)) + { + File.Delete(schemaName); + } + } + + /// + /// Creates a hot reload scenario in which the updated configuration file is invalid causing + /// hot reload to fail as the schema can't be used by DAB, then we check that the + /// program is still able to work properly by showing us that it is still using the + /// same configuration file from before the hot reload. + /// + [TestMethod] + [TestCategory(TestCategory.MSSQL)] + public void HotReloadParsingFail() + { + // Arrange + string schemaName = "dab.draft.schema.json"; + string configName = "hotreload-config.json"; + if (File.Exists(configName)) + { + File.Delete(configName); + } + + bool initialRestEnabled = true; + + bool initialGQLEnabled = true; + + DataSource dataSource = new(DatabaseType.MSSQL, + ConfigurationTests.GetConnectionStringFromEnvironmentConfig(environment: TestCategory.MSSQL), Options: null); + + RuntimeConfig initialConfig = InitRuntimeConfigForHotReload(schemaName, dataSource, initialRestEnabled, initialGQLEnabled); + + string updatedConfig = TestHelper.GenerateInvalidRuntimeSection(); + + // Not using mocked filesystem so we pick up real file changes for hot reload + FileSystem fileSystem = new(); + RuntimeConfigProvider configProvider = GenerateConfigFileAndConfigProvider(fileSystem, configName, initialConfig); + RuntimeConfig lkgRuntimeConfig = configProvider.GetConfig(); + + Assert.IsNotNull(lkgRuntimeConfig); + + // Act + // Simulate an invalid change to the config file + fileSystem.File.WriteAllText(configName, updatedConfig); + + // Give ConfigFileWatcher enough time to hot reload the change + System.Threading.Thread.Sleep(1000); + + RuntimeConfig newRuntimeConfig = configProvider.GetConfig(); + Assert.AreEqual(expected: lkgRuntimeConfig, actual: newRuntimeConfig); + + if (File.Exists(configName)) + { + File.Delete(configName); + } + } + /// /// Helper function to write custom configuration file. with minimal REST/GraphQL global settings /// using the supplied entities. @@ -4596,6 +4715,53 @@ public static RuntimeConfig InitMinimalRuntimeConfig( ); } + /// + /// Helper function that initializes a RuntimeConfig with the following + /// variables in order to prepare a file for hot reload + /// + /// + /// + /// + /// + /// + public static RuntimeConfig InitRuntimeConfigForHotReload( + string schema, + DataSource dataSource, + bool restEnabled, + bool graphQLEnabled) + { + RuntimeConfig runtimeConfig = new( + Schema: schema, + DataSource: dataSource, + Runtime: new( + Rest: new(restEnabled), + GraphQL: new(graphQLEnabled), + Host: new(null, null, HostMode.Development) + ), + Entities: new(new Dictionary()) + ); + + return runtimeConfig; + } + + /// + /// Helper function that generates a config file that is going to be observed + /// for hot reload and initialize a ConfigProvider which will be used to check + /// if the hot reload was validated or not. + /// + /// + /// + /// + /// + public static RuntimeConfigProvider GenerateConfigFileAndConfigProvider(FileSystem fileSystem, string configName, RuntimeConfig runtimeConfig) + { + fileSystem.File.WriteAllText(configName, runtimeConfig.ToJson()); + FileSystemRuntimeConfigLoader configLoader = new(fileSystem, handler: null, configName, string.Empty); + RuntimeConfigProvider configProvider = new(configLoader); + + return configProvider; + } + /// /// Gets PermissionSetting object allowed to perform all actions. /// diff --git a/src/Service.Tests/TestHelper.cs b/src/Service.Tests/TestHelper.cs index a8d152651c..234dea2c46 100644 --- a/src/Service.Tests/TestHelper.cs +++ b/src/Service.Tests/TestHelper.cs @@ -108,6 +108,47 @@ public static RuntimeConfig AddMissingEntitiesToConfig(RuntimeConfig config, str return config with { Entities = new(entities) }; } + /// + /// This function generates an invalid config file that is used by the tests + /// to ensure that parsing of the config file fails during a hot reload event + /// + /// + public static string GenerateInvalidRuntimeSection() + { + string runtimeString = @" +{ + ""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"", + ""data-source"": { + ""database-type"": ""mssql"", + ""connection-string"": ""Server=test;Database=test;User ID=test;"", + ""options"": { + ""set-session-context"": true + } + }, + ""runtime"": { + ""rest"": { + ""enabled"": +}"; + return runtimeString; + } + + /// + /// This function generates an invalid schema file that is used by the tests + /// to ensure that validation of the config file fails during a hot reload event + /// + /// + public static string GenerateInvalidSchema() + { + string schemaString = @" +{ + ""$schema"": ""https://json-schema.org/draft-07/schema"", + ""$id"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch/dab.draft.schema.json"", + ""title"": ""Data API builder"", + ""description"": ""Schema for +}"; + return schemaString; + } + /// /// Schema property of the config json. This is used for constructing the required config json strings /// for unit tests diff --git a/src/Service/Azure.DataApiBuilder.Service.csproj b/src/Service/Azure.DataApiBuilder.Service.csproj index 56d14139d3..df3b648370 100644 --- a/src/Service/Azure.DataApiBuilder.Service.csproj +++ b/src/Service/Azure.DataApiBuilder.Service.csproj @@ -92,4 +92,14 @@ + + + + + + + PreserveNewest + + +