Handling missing environment variables cleaner for SWA (#1825)

## Why make this change?

- Fixes #1820 

## What is this change?

- New test coverage for #1820
- Added a way to control error handling when env vars aren't set -
default is left to throw, override to ignore

## How was this tested?

- [x] Integration Tests
- [ ] Unit Tests

src/Cli.Tests/EnvironmentTests.cs

@@ -19,7 +19,7 @@ public class EnvironmentTests
     [TestInitialize]
     public void TestInitialize()
     {
-        StringJsonConverterFactory converterFactory = new();
+        StringJsonConverterFactory converterFactory = new(EnvironmentVariableReplacementFailureMode.Throw);
         _options = new()
         {
             PropertyNameCaseInsensitive = true

src/Config/Converters/EnvironmentVariableReplacementFailureMode.cs

@@ -0,0 +1,19 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace Azure.DataApiBuilder.Config.Converters;
+
+/// <summary>
+/// Control how to handle environment variable replacement failures when deserializing strings in the JSON config file.
+/// </summary>
+public enum EnvironmentVariableReplacementFailureMode
+{
+    /// <summary>
+    /// Ignore the missing environment variable and return the original value, eg: @env('schema').
+    /// </summary>
+    Ignore,
+    /// <summary>
+    /// Throw an exception when a missing environment variable is encountered. This is the default behavior.
+    /// </summary>
+    Throw
+}

src/Config/Converters/StringJsonConverterFactory.cs

@@ -14,14 +14,21 @@ namespace Azure.DataApiBuilder.Config.Converters;
 /// </summary>
 public class StringJsonConverterFactory : JsonConverterFactory
 {
+    private EnvironmentVariableReplacementFailureMode _replacementFailureMode;
+
+    public StringJsonConverterFactory(EnvironmentVariableReplacementFailureMode replacementFailureMode)
+    {
+        _replacementFailureMode = replacementFailureMode;
+    }
+
     public override bool CanConvert(Type typeToConvert)
     {
         return typeToConvert.IsAssignableTo(typeof(string));
     }
 
     public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
     {
-        return new StringJsonConverter();
+        return new StringJsonConverter(_replacementFailureMode);
     }
 
     class StringJsonConverter : JsonConverter<string>
@@ -42,6 +49,12 @@ class StringJsonConverter : JsonConverter<string>
         // within the name of the environment variable, but that ') is not
         // a valid environment variable name in certain shells.
         const string ENV_PATTERN = @"@env\('.*?(?='\))'\)";
+        private EnvironmentVariableReplacementFailureMode _replacementFailureMode;
+
+        public StringJsonConverter(EnvironmentVariableReplacementFailureMode replacementFailureMode)
+        {
+            _replacementFailureMode = replacementFailureMode;
+        }
 
         public override string? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
@@ -64,7 +77,7 @@ public override void Write(Utf8JsonWriter writer, string value, JsonSerializerOp
             writer.WriteStringValue(value);
         }
 
-        private static string ReplaceMatchWithEnvVariable(Match match)
+        private string ReplaceMatchWithEnvVariable(Match match)
         {
             // [^@env\(]   :  any substring that is not @env(
             // .*          :  any char except newline any number of times
@@ -76,10 +89,17 @@ private static string ReplaceMatchWithEnvVariable(Match match)
             // strips first and last characters, ie: '''hello'' --> ''hello'
             string envName = Regex.Match(match.Value, innerPattern).Value[1..^1];
             string? envValue = Environment.GetEnvironmentVariable(envName);
-            return envValue is not null ? envValue :
-                throw new DataApiBuilderException(message: $"Environmental Variable, {envName}, not found.",
-                                               statusCode: System.Net.HttpStatusCode.ServiceUnavailable,
-                                               subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization);
+            if (_replacementFailureMode == EnvironmentVariableReplacementFailureMode.Throw)
+            {
+                return envValue is not null ? envValue :
+                    throw new DataApiBuilderException(message: $"Environmental Variable, {envName}, not found.",
+                                                   statusCode: System.Net.HttpStatusCode.ServiceUnavailable,
+                                                   subStatusCode: DataApiBuilderException.SubStatusCodes.ErrorInInitialization);
+            }
+            else
+            {
+                return envValue ?? match.Value;
+            }
         }
     }
 }

src/Config/Converters/Utf8JsonReaderExtensions.cs

@@ -15,9 +15,12 @@ static internal class Utf8JsonReaderExtensions
     /// <param name="reader">The reader that we want to pull the string from.</param>
     /// <param name="replaceEnvVar">Whether to replace environment variable with its
     /// value or not while deserializing.</param>
+    /// <param name="replacementFailureMode">The failure mode to use when replacing environment variables.</param>
     /// <returns>The result of deserialization.</returns>
     /// <exception cref="JsonException">Thrown if the <see cref="JsonTokenType"/> is not String.</exception>
-    public static string? DeserializeString(this Utf8JsonReader reader, bool replaceEnvVar)
+    public static string? DeserializeString(this Utf8JsonReader reader,
+        bool replaceEnvVar,
+        EnvironmentVariableReplacementFailureMode replacementFailureMode = EnvironmentVariableReplacementFailureMode.Throw)
     {
         if (reader.TokenType is JsonTokenType.Null)
         {
@@ -33,7 +36,7 @@ static internal class Utf8JsonReaderExtensions
         JsonSerializerOptions options = new();
         if (replaceEnvVar)
         {
-            options.Converters.Add(new StringJsonConverterFactory());
+            options.Converters.Add(new StringJsonConverterFactory(replacementFailureMode));
         }
 
         return JsonSerializer.Deserialize<string>(ref reader, options);

src/Config/RuntimeConfigLoader.cs

@@ -60,9 +60,10 @@ public static bool TryParseConfig(string json,
         string? connectionString = null,
         bool replaceEnvVar = false,
         string dataSourceName = "",
-        Dictionary<string, string>? datasourceNameToConnectionString = null)
+        Dictionary<string, string>? datasourceNameToConnectionString = null,
+        EnvironmentVariableReplacementFailureMode replacementFailureMode = EnvironmentVariableReplacementFailureMode.Throw)
     {
-        JsonSerializerOptions options = GetSerializationOptions(replaceEnvVar);
+        JsonSerializerOptions options = GetSerializationOptions(replaceEnvVar, replacementFailureMode);
 
         try
         {
@@ -171,7 +172,9 @@ public static bool TryParseConfig(string json,
     /// </summary>
     /// <param name="replaceEnvVar">Whether to replace environment variable with value or not while deserializing.
     /// By default, no replacement happens.</param>
-    public static JsonSerializerOptions GetSerializationOptions(bool replaceEnvVar = false)
+    public static JsonSerializerOptions GetSerializationOptions(
+        bool replaceEnvVar = false,
+        EnvironmentVariableReplacementFailureMode replacementFailureMode = EnvironmentVariableReplacementFailureMode.Throw)
     {
 
         JsonSerializerOptions options = new()
@@ -193,7 +196,7 @@ public static JsonSerializerOptions GetSerializationOptions(bool replaceEnvVar =
 
         if (replaceEnvVar)
         {
-            options.Converters.Add(new StringJsonConverterFactory());
+            options.Converters.Add(new StringJsonConverterFactory(replacementFailureMode));
         }
 
         return options;

src/Core/Configurations/RuntimeConfigProvider.cs

@@ -6,6 +6,7 @@
 using System.Net;
 using System.Text.Json;
 using Azure.DataApiBuilder.Config;
+using Azure.DataApiBuilder.Config.Converters;
 using Azure.DataApiBuilder.Config.NamingPolicies;
 using Azure.DataApiBuilder.Config.ObjectModel;
 using Azure.DataApiBuilder.Service.Exceptions;
@@ -136,7 +137,8 @@ public async Task<bool> Initialize(
         if (RuntimeConfigLoader.TryParseConfig(
                 configuration,
                 out RuntimeConfig? runtimeConfig,
-                replaceEnvVar: true))
+                replaceEnvVar: false,
+                replacementFailureMode: EnvironmentVariableReplacementFailureMode.Ignore))
         {
             _runtimeConfig = runtimeConfig;
 
@@ -170,7 +172,13 @@ public async Task<bool> Initialize(
     /// <param name="connectionString">The connection string to the database.</param>
     /// <param name="accessToken">The string representation of a managed identity access token</param>
     /// <returns>true if the initialization succeeded, false otherwise.</returns>
-    public async Task<bool> Initialize(string jsonConfig, string? graphQLSchema, string connectionString, string? accessToken)
+    public async Task<bool> Initialize(
+        string jsonConfig,
+        string? graphQLSchema,
+        string connectionString,
+        string? accessToken,
+        bool replaceEnvVar = true,
+        EnvironmentVariableReplacementFailureMode replacementFailureMode = EnvironmentVariableReplacementFailureMode.Throw)
     {
         if (string.IsNullOrEmpty(connectionString))
         {
@@ -184,7 +192,7 @@ public async Task<bool> Initialize(string jsonConfig, string? graphQLSchema, str
 
         IsLateConfigured = true;
 
-        if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, replaceEnvVar: true))
+        if (RuntimeConfigLoader.TryParseConfig(jsonConfig, out RuntimeConfig? runtimeConfig, replaceEnvVar: replaceEnvVar, replacementFailureMode: replacementFailureMode))
         {
             _runtimeConfig = runtimeConfig.DataSource.DatabaseType switch
             {

src/Service.Tests/Configuration/ConfigurationEndpoints.cs

@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace Azure.DataApiBuilder.Service.Tests.Configuration;
+
+internal static class ConfigurationEndpoints
+{
+    // TODO: Remove the old endpoint once we've updated all callers to use the new one.
+    public const string CONFIGURATION_ENDPOINT = "/configuration";
+    public const string CONFIGURATION_ENDPOINT_V2 = "/configuration/v2";
+}

src/Service.Tests/Configuration/ConfigurationTests.cs

@@ -43,6 +43,8 @@
 using Npgsql;
 using VerifyMSTest;
 using static Azure.DataApiBuilder.Config.FileSystemRuntimeConfigLoader;
+using static Azure.DataApiBuilder.Service.Tests.Configuration.ConfigurationEndpoints;
+using static Azure.DataApiBuilder.Service.Tests.Configuration.TestConfigFileReader;
 
 namespace Azure.DataApiBuilder.Service.Tests.Configuration
 {
@@ -67,10 +69,6 @@ public class ConfigurationTests
         private const int RETRY_COUNT = 5;
         private const int RETRY_WAIT_SECONDS = 1;
 
-        // TODO: Remove the old endpoint once we've updated all callers to use the new one.
-        private const string CONFIGURATION_ENDPOINT = "/configuration";
-        private const string CONFIGURATION_ENDPOINT_V2 = "/configuration/v2";
-
         /// <summary>
         /// A valid REST API request body with correct parameter types for all the fields.
         /// </summary>
@@ -2232,7 +2230,7 @@ private static JsonContent GetJsonContentForCosmosConfigRequest(string endpoint,
             if (CONFIGURATION_ENDPOINT == endpoint)
             {
                 ConfigurationPostParameters configParams = GetCosmosConfigurationParameters();
-                if (config != null)
+                if (config is not null)
                 {
                     configParams = configParams with { Configuration = config };
                 }
@@ -2329,21 +2327,6 @@ private static ConfigurationPostParametersV2 GetCosmosConfigurationParametersV2(
                 AccessToken: null);
         }
 
-        private static RuntimeConfig ReadCosmosConfigurationFromFile()
-        {
-            string cosmosFile = $"{CONFIGFILE_NAME}.{COSMOS_ENVIRONMENT}{CONFIG_EXTENSION}";
-
-            string configurationFileContents = File.ReadAllText(cosmosFile);
-            if (!RuntimeConfigLoader.TryParseConfig(configurationFileContents, out RuntimeConfig config))
-            {
-                throw new Exception("Failed to parse configuration file.");
-            }
-
-            // The Schema file isn't provided in the configuration file when going through the configuration endpoint so we're removing it.
-            config.DataSource.Options.Remove("Schema");
-            return config;
-        }
-
         /// <summary>
         /// Helper used to create the post-startup configuration payload sent to configuration controller.
         /// Adds entity used to hydrate authorization resolver post-startup and validate that hydration succeeds.

src/Service.Tests/Configuration/LoadConfigViaEndpoint.cs

@@ -0,0 +1,110 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.Net;
+using System.Net.Http;
+using System.Net.Http.Json;
+using System.Threading.Tasks;
+using Azure.DataApiBuilder.Config.ObjectModel;
+using Azure.DataApiBuilder.Core.Configurations;
+using Azure.DataApiBuilder.Service.Controllers;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using static Azure.DataApiBuilder.Service.Tests.Configuration.ConfigurationEndpoints;
+using static Azure.DataApiBuilder.Service.Tests.Configuration.TestConfigFileReader;
+
+namespace Azure.DataApiBuilder.Service.Tests.Configuration;
+
+[TestClass]
+public class LoadConfigViaEndpointTests
+{
+    [TestMethod("Testing that missing environment variables won't cause runtime failure."), TestCategory(TestCategory.COSMOSDBNOSQL)]
+    [DataRow(CONFIGURATION_ENDPOINT)]
+    [DataRow(CONFIGURATION_ENDPOINT_V2)]
+    public async Task CanLoadConfigWithMissingEnvironmentVariables(string configurationEndpoint)
+    {
+        TestServer server = new(Program.CreateWebHostFromInMemoryUpdateableConfBuilder(Array.Empty<string>()));
+        HttpClient httpClient = server.CreateClient();
+
+        (RuntimeConfig config, JsonContent content) = GetParameterContent(configurationEndpoint);
+
+        HttpResponseMessage postResult =
+            await httpClient.PostAsync(configurationEndpoint, content);
+        Assert.AreEqual(HttpStatusCode.OK, postResult.StatusCode);
+
+        RuntimeConfigProvider configProvider = server.Services.GetService(typeof(RuntimeConfigProvider)) as RuntimeConfigProvider;
+        RuntimeConfig loadedConfig = configProvider.GetConfig();
+
+        Assert.AreEqual(config.Schema, loadedConfig.Schema);
+    }
+
+    [TestMethod("Testing that environment variables can be replaced at runtime not only when config is loaded."), TestCategory(TestCategory.COSMOSDBNOSQL)]
+    [DataRow(CONFIGURATION_ENDPOINT)]
+    [DataRow(CONFIGURATION_ENDPOINT_V2)]
+    [Ignore("We don't want to environment variable substitution in late configuration, but test is left in for if this changes.")]
+    public async Task CanLoadConfigWithEnvironmentVariables(string configurationEndpoint)
+    {
+        Environment.SetEnvironmentVariable("schema", "schema.graphql");
+        TestServer server = new(Program.CreateWebHostFromInMemoryUpdateableConfBuilder(Array.Empty<string>()));
+        HttpClient httpClient = server.CreateClient();
+
+        (RuntimeConfig config, JsonContent content) = GetParameterContent(configurationEndpoint);
+
+        HttpResponseMessage postResult =
+            await httpClient.PostAsync(configurationEndpoint, content);
+        Assert.AreEqual(HttpStatusCode.OK, postResult.StatusCode);
+
+        RuntimeConfigProvider configProvider = server.Services.GetService(typeof(RuntimeConfigProvider)) as RuntimeConfigProvider;
+        RuntimeConfig loadedConfig = configProvider.GetConfig();
+
+        Assert.AreNotEqual(config.Schema, loadedConfig.Schema);
+        Assert.AreEqual(Environment.GetEnvironmentVariable("schema"), loadedConfig.Schema);
+    }
+
+    [TestCleanup]
+    public void Cleanup()
+    {
+        Environment.SetEnvironmentVariable("schema", null);
+    }
+
+    private static (RuntimeConfig, JsonContent) GetParameterContent(string endpoint)
+    {
+        RuntimeConfig config = ReadCosmosConfigurationFromFile() with { Schema = "@env('schema')" };
+
+        if (endpoint == CONFIGURATION_ENDPOINT)
+        {
+            ConfigurationPostParameters @params = new(
+                Configuration: config.ToJson(),
+                Schema: @"
+                type Entity {
+                    id: ID!
+                    name: String!
+                }
+                ",
+                ConnectionString: "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==",
+                AccessToken: null
+            );
+
+            return (config, JsonContent.Create(@params));
+        }
+        else if (endpoint == CONFIGURATION_ENDPOINT_V2)
+        {
+            ConfigurationPostParametersV2 @params = new(
+                Configuration: config.ToJson(),
+                ConfigurationOverrides: "{}",
+                Schema: @"
+                type Entity {
+                    id: ID!
+                    name: String!
+                }
+                ",
+                AccessToken: null
+            );
+
+            return (config, JsonContent.Create(@params));
+        }
+
+        throw new ArgumentException($"Unknown endpoint: {endpoint}");
+    }
+}

src/Service.Tests/Configuration/TestConfigFileReader.cs

@@ -0,0 +1,31 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.IO;
+using Azure.DataApiBuilder.Config;
+using Azure.DataApiBuilder.Config.ObjectModel;
+using static Azure.DataApiBuilder.Config.FileSystemRuntimeConfigLoader;
+
+namespace Azure.DataApiBuilder.Service.Tests.Configuration;
+
+/// <summary>
+/// Provides the methods to read the configuration files from disk for tests.
+/// </summary>
+internal static class TestConfigFileReader
+{
+    public static RuntimeConfig ReadCosmosConfigurationFromFile()
+    {
+        string cosmosFile = $"{CONFIGFILE_NAME}.{TestCategory.COSMOSDBNOSQL}{CONFIG_EXTENSION}";
+
+        string configurationFileContents = File.ReadAllText(cosmosFile);
+        if (!RuntimeConfigLoader.TryParseConfig(configurationFileContents, out RuntimeConfig config))
+        {
+            throw new Exception("Failed to parse configuration file.");
+        }
+
+        // The Schema file isn't provided in the configuration file when going through the configuration endpoint so we're removing it.
+        _ = config.DataSource.Options.Remove("Schema");
+        return config;
+    }
+}