diff --git a/.gitattributes b/.gitattributes index a224f6bcbb..8d16315cf8 100644 --- a/.gitattributes +++ b/.gitattributes @@ -15,3 +15,5 @@ *.verified.txt text eol=lf working-tree-encoding=UTF-8 *.verified.xml text eol=lf working-tree-encoding=UTF-8 *.verified.json text eol=lf working-tree-encoding=UTF-8 + +*.cs text eol=lf diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ab98fbe8cf..a9fd633f27 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -95,7 +95,7 @@ We use `dotnet format` to enforce code conventions. It is run automatically in C #### Enforcing code style with git hooks -You can copy paste the following commands to install a git pre-commit hook. This will cause a commit to fail if you forgot to run `dotnet format`. If you have run on save enabled in your editor this is not necessary. +You can copy paste the following commands to install a git pre-commit hook (creates a pre-commit file in your .git folder, which isn't shown in vs code). This will cause a commit to fail if you forgot to run `dotnet format`. If you have run on save enabled in your editor this is not necessary. ```bash cat > .git/hooks/pre-commit << __EOF__ @@ -112,17 +112,42 @@ if [ "\$(get_files)" = '' ]; then fi get_files | - xargs dotnet format src/Azure.DataApiBuilder.Service.sln \\ - --check \\ - --fix-whitespace --fix-style warn --fix-analyzers warn \\ + xargs dotnet format src/Azure.DataApiBuilder.sln \\ + --verify-no-changes --include \\ || { get_files | - xargs dotnet format src/Azure.DataApiBuilder.Service.sln \\ - --fix-whitespace --fix-style warn --fix-analyzers warn \\ + xargs dotnet format src/Azure.DataApiBuilder.sln \\ --include exit 1 } __EOF__ chmod +x .git/hooks/pre-commit ``` + +The file should look like this + +``` bash +#!/bin/bash +set -euo pipefail + +get_files() { + git diff --cached --name-only --diff-filter=ACMR | \ + grep '\.cs$' +} + +if [ "$(get_files)" = '' ]; then + exit 0 +fi + +get_files | + xargs dotnet format src/Azure.DataApiBuilder.sln \ + --verify-no-changes \ + --include \ + || { + get_files | + xargs dotnet format src/Azure.DataApiBuilder.sln \ + --include + exit 1 +} +``` diff --git a/schemas/dab.draft.schema.json b/schemas/dab.draft.schema.json index b684cc28ac..920c0a4da6 100644 --- a/schemas/dab.draft.schema.json +++ b/schemas/dab.draft.schema.json @@ -41,8 +41,8 @@ "additionalProperties": false, "properties": { "enabled": { - "type": "boolean", - "description": "Enable health check endpoint", + "$ref": "#/$defs/boolean-or-string", + "description": "Enable health check endpoint for something", "default": true, "additionalProperties": false }, @@ -186,7 +186,7 @@ "type": "string" }, "enabled": { - "type": "boolean", + "$ref": "#/$defs/boolean-or-string", "description": "Allow enabling/disabling REST requests for all entities." }, "request-body-strict": { @@ -210,7 +210,7 @@ "type": "string" }, "enabled": { - "type": "boolean", + "$ref": "#/$defs/boolean-or-string", "description": "Allow enabling/disabling GraphQL requests for all entities." }, "depth-limit": { @@ -438,7 +438,7 @@ "description": "Application Insights connection string" }, "enabled": { - "type": "boolean", + "$ref": "#/$defs/boolean-or-string", "description": "Allow enabling/disabling Application Insights telemetry.", "default": true } @@ -481,7 +481,7 @@ "additionalProperties": false, "properties": { "enabled": { - "type": "boolean", + "$ref": "#/$defs/boolean-or-string", "description": "Allow enabling/disabling Azure Log Analytics.", "default": false }, @@ -618,7 +618,7 @@ "additionalProperties": false, "properties": { "enabled": { - "type": "boolean", + "$ref": "#/$defs/boolean-or-string", "description": "Enable health check endpoint globally", "default": true, "additionalProperties": false @@ -1391,7 +1391,15 @@ "type": "string" } }, - "required": ["singular"] + "required": [ "singular" ] + } + ] + }, + "boolean-or-string": { + "oneOf":[ + { + "type": [ "boolean", "string" ], + "pattern": "^(?:true|false|1|0|@env\\('.*'\\)|@akv\\('.*'\\))$" } ] }, diff --git a/src/Config/Converters/BooleanJsonConverter.cs b/src/Config/Converters/BooleanJsonConverter.cs new file mode 100644 index 0000000000..a85eb6fc74 --- /dev/null +++ b/src/Config/Converters/BooleanJsonConverter.cs @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Text.Json; +using System.Text.Json.Serialization; + +namespace Azure.DataApiBuilder.Config.Converters; + +/// +/// JSON converter for boolean values that also supports string representations such as +/// "true", "false", "1", and "0". Any environment variable replacement is handled by +/// other converters (for example, the string converter) before the value is parsed here. +public class BoolJsonConverter : JsonConverter +{ + + public override bool Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) + { + if (reader.TokenType is JsonTokenType.Null) + { + + throw new JsonException("Unexpected null JSON token. Expected a boolean literal or a valid @expression."); + } + + if (reader.TokenType == JsonTokenType.String) + { + + string? tempBoolean = JsonSerializer.Deserialize(ref reader, options); + + bool result = tempBoolean?.ToLower() switch + { + //numeric values have to be checked here as they may come from string replacement + "true" or "1" => true, + "false" or "0" => false, + _ => throw new JsonException($"Invalid boolean value: {tempBoolean}. Specify either true or false."), + }; + + return result; + } + else if (reader.TokenType == JsonTokenType.Number) + { + bool result = reader.GetInt32() switch + { + 1 => true, + 0 => false, + _ => throw new JsonException($"Invalid boolean value. Specify either true or false."), + }; + return result; + } + else + { + return reader.GetBoolean(); + } + + throw new JsonException("Invalid JSON value. Expected a boolean literal or a valid @expression."); + } + + public override void Write(Utf8JsonWriter writer, bool value, JsonSerializerOptions options) + { + writer.WriteBooleanValue(value); + } +} diff --git a/src/Config/RuntimeConfigLoader.cs b/src/Config/RuntimeConfigLoader.cs index 5996a902ed..9a54d09d8e 100644 --- a/src/Config/RuntimeConfigLoader.cs +++ b/src/Config/RuntimeConfigLoader.cs @@ -327,6 +327,7 @@ public static JsonSerializerOptions GetSerializationOptions( options.Converters.Add(new AKVRetryPolicyOptionsConverterFactory(replacementSettings)); options.Converters.Add(new AzureLogAnalyticsOptionsConverterFactory(replacementSettings)); options.Converters.Add(new AzureLogAnalyticsAuthOptionsConverter(replacementSettings)); + options.Converters.Add(new BoolJsonConverter()); options.Converters.Add(new FileSinkConverter(replacementSettings)); // Add AzureKeyVaultOptionsConverterFactory to ensure AKV config is deserialized properly diff --git a/src/Service.Tests/Caching/CachingConfigProcessingTests.cs b/src/Service.Tests/Caching/CachingConfigProcessingTests.cs index 00729476cd..1294c009da 100644 --- a/src/Service.Tests/Caching/CachingConfigProcessingTests.cs +++ b/src/Service.Tests/Caching/CachingConfigProcessingTests.cs @@ -161,8 +161,6 @@ public void GlobalCacheOptionsDeserialization_ValidValues( [DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": 0 }", DisplayName = "EntityCacheOptions.TtlSeconds set to zero is invalid configuration.")] [DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": -1 }", DisplayName = "EntityCacheOptions.TtlSeconds set to negative number is invalid configuration.")] [DataRow(@",""cache"": { ""enabled"": true, ""ttl-seconds"": 1.1 }", DisplayName = "EntityCacheOptions.TtlSeconds set to decimal is invalid configuration.")] - [DataRow(@",""cache"": { ""enabled"": 1 }", DisplayName = "EntityCacheOptions.Enabled property set to 1 should fail because not a boolean.")] - [DataRow(@",""cache"": { ""enabled"": 0 }", DisplayName = "EntityCacheOptions.Enabled property set to 0 should fail because not a boolean.")] [DataRow(@",""cache"": 1", DisplayName = "EntityCacheOptions property set to 1 should fail because it's not a JSON object.")] [DataRow(@",""cache"": 0", DisplayName = "EntityCacheOptions property set to 0 should fail because it's not a JSON object.")] [DataRow(@",""cache"": true", DisplayName = "EntityCacheOptions property set to true should fail because it's not a JSON object.")] diff --git a/src/Service.Tests/Configuration/ConfigurationTests.cs b/src/Service.Tests/Configuration/ConfigurationTests.cs index 1faa8eb08a..f6650dd349 100644 --- a/src/Service.Tests/Configuration/ConfigurationTests.cs +++ b/src/Service.Tests/Configuration/ConfigurationTests.cs @@ -664,6 +664,46 @@ type Moon { ""entities"":{ } }"; + public const string CONFIG_FILE_WITH_BOOLEAN_AS_ENV = @"{ + // Link for latest draft schema. + ""$schema"":""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch-alpha/dab.draft.schema.json"", + ""data-source"": { + ""database-type"": ""mssql"", + ""connection-string"": ""sample-conn-string"", + ""health"": { + ""enabled"": + } + }, + ""runtime"": { + ""health"": { + ""enabled"": + }, + ""rest"": { + ""enabled"": , + ""path"": ""/api"" + }, + ""graphql"": { + ""enabled"": , + ""path"": ""/graphql"", + ""allow-introspection"": true + }, + ""host"": { + ""authentication"": { + ""provider"": ""AppService"" + } + }, + ""telemetry"": { + ""application-insights"":{ + ""enabled"": , + ""connection-string"":""sample-ai-connection-string"" + } + + } + + }, + ""entities"":{ } + }"; + [TestCleanup] public void CleanupAfterEachTest() { @@ -1820,8 +1860,44 @@ public void TestBasicConfigSchemaWithNoOptionalFieldsIsValid(string jsonData) JsonConfigSchemaValidator jsonSchemaValidator = new(schemaValidatorLogger.Object, new MockFileSystem()); JsonSchemaValidationResult result = jsonSchemaValidator.ValidateJsonConfigWithSchema(jsonSchema, jsonData); - Assert.IsTrue(result.IsValid); + Assert.AreEqual("", String.Join('\n', result.ValidationErrors?.Select(s => $"{s.Message} at {s.Path} {s.LineNumber} {s.LinePosition}") ?? []), "Expected no validation errors."); Assert.IsTrue(EnumerableUtilities.IsNullOrEmpty(result.ValidationErrors)); + + Assert.IsTrue(result.IsValid); + schemaValidatorLogger.Verify( + x => x.Log( + LogLevel.Information, + It.IsAny(), + It.Is((o, t) => o.ToString()!.Contains($"The config satisfies the schema requirements.")), + It.IsAny(), + (Func)It.IsAny()), + Times.Once); + } + + [DataTestMethod] + [DataRow("true", DisplayName = "Validates variable boolean schema for true value")] + [DataRow("false", DisplayName = "Validates variable boolean schema for false value.")] + [DataRow("\"true\"", DisplayName = "Validates variable boolean schema for true as string.")] + [DataRow("\"false\"", DisplayName = "Validates variable boolean schema for false as string.")] + [DataRow("\"1\"", DisplayName = "Validates variable boolean schema for 1 as string.")] + [DataRow("\"0\"", DisplayName = "Validates variable boolean schema for 0as string.")] + [DataRow("\"@env('SAMPLE')\"", DisplayName = "Validates variable boolean schema for environment variables.")] + [DataRow("\"@akv('SAMPLE')\"", DisplayName = "Validates variable boolean schema for keyvaul variables.")] + public void TestBasicConfigSchemaWithFlexibleBoolean(string Value) + { + Mock> schemaValidatorLogger = new(); + + string jsonSchema = File.ReadAllText("dab.draft.schema.json"); + + JsonConfigSchemaValidator jsonSchemaValidator = new(schemaValidatorLogger.Object, new MockFileSystem()); + + string jsonData = CONFIG_FILE_WITH_BOOLEAN_AS_ENV.Replace("", Value); + JsonSchemaValidationResult result = jsonSchemaValidator.ValidateJsonConfigWithSchema(jsonSchema, jsonData); + Assert.AreEqual("", String.Join('\n', result.ValidationErrors?.Select(s => $"{s.Message} at {s.Path} {s.LineNumber} {s.LinePosition}") ?? []), "Expected no validation errors."); + + Assert.IsTrue(EnumerableUtilities.IsNullOrEmpty(result.ValidationErrors), "Validation Erros null of empty"); + + Assert.IsTrue(result.IsValid, "Result should be valid"); schemaValidatorLogger.Verify( x => x.Log( LogLevel.Information, @@ -3368,7 +3444,7 @@ public async Task ValidateStrictModeAsDefaultForRestRequestBody(bool includeExtr HttpMethod httpMethod = SqlTestHelper.ConvertRestMethodToHttpMethod(SupportedHttpVerb.Post); string requestBody = @"{ ""title"": ""Harry Potter and the Order of Phoenix"", - ""publisher_id"": 1234"; + ""publisher_id"": 1234 }"; if (includeExtraneousFieldInRequestBody) { diff --git a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs index 2a0e49cf00..1b06d3cea9 100644 --- a/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs +++ b/src/Service.Tests/UnitTests/RuntimeConfigLoaderJsonDeserializerTests.cs @@ -14,7 +14,9 @@ using Azure.DataApiBuilder.Config.ObjectModel; using Azure.DataApiBuilder.Service.Exceptions; using Microsoft.Data.SqlClient; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; namespace Azure.DataApiBuilder.Service.Tests.UnitTests { @@ -213,6 +215,138 @@ private static string GetExpectedPropertyValue(string envVarName, bool replaceEn } } + /// + /// Test method to validate that environment variable replacement works correctly + /// for the telemetry.application-insights.enabled property when set through config + /// or through environment variables + /// + [TestMethod] + [DataRow(true, DisplayName = "ApplicationInsights.Enabled set to true (literal bool)")] + [DataRow(false, DisplayName = "ApplicationInsights.Enabled set to false (literal bool)")] + public void TestTelemetryApplicationInsightsEnabled(bool expected) + { + TestTelemetryApplicationInsightsEnabledInternal(expected.ToString().ToLower(), expected); + } + + [TestMethod] + [DataRow("true", true, DisplayName = "ApplicationInsights.Enabled from string 'true'")] + [DataRow("false", false, DisplayName = "ApplicationInsights.Enabled from string 'false'")] + [DataRow("1", true, DisplayName = "ApplicationInsights.Enabled from string '1'")] + [DataRow("0", false, DisplayName = "ApplicationInsights.Enabled from string '0'")] + public void TestTelemetryApplicationInsightsEnabledFromString(string configSetting, bool expected) + { + + TestTelemetryApplicationInsightsEnabledInternal($"\"{configSetting}\"", expected); + } + + [TestMethod] + [DataRow("true", true, DisplayName = "ApplicationInsights.Enabled from environment 'true'")] + [DataRow("false", false, DisplayName = "ApplicationInsights.Enabled from environment 'false'")] + [DataRow("1", true, DisplayName = "ApplicationInsights.Enabled from environment '1'")] + [DataRow("0", false, DisplayName = "ApplicationInsights.Enabled from environment '0'")] + public void TestTelemetryApplicationInsightsEnabledFromEnvironment(string configSetting, bool expected) + { + // Arrange + const string envVarName = "APP_INSIGHTS_ENABLED"; + string envVarValue = configSetting; + // Set up the environment variable + Environment.SetEnvironmentVariable(envVarName, envVarValue); + + try + { + TestTelemetryApplicationInsightsEnabledInternal("\"@env('APP_INSIGHTS_ENABLED')\"", expected); + } + finally + { + // Cleanup + Environment.SetEnvironmentVariable(envVarName, null); + } + + } + public static void TestTelemetryApplicationInsightsEnabledInternal(string configValue, bool expected) + { + string configJson = @"{ + ""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch-alpha/dab.draft.schema.json"", + ""data-source"": { + ""database-type"": ""mssql"", + ""connection-string"": ""Server=tcp:127.0.0.1,1433;Persist Security Info=False;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=False;Connection Timeout=5;"" + }, + ""runtime"": { + ""telemetry"": { + ""application-insights"": { + ""enabled"": " + configValue + @", + ""connection-string"": ""InstrumentationKey=test-key"" + } + } + }, + ""entities"": { } + }"; + + // Act + bool IsParsed = RuntimeConfigLoader.TryParseConfig( + configJson, + out RuntimeConfig runtimeConfig, + replacementSettings: new DeserializationVariableReplacementSettings( + azureKeyVaultOptions: null, + doReplaceEnvVar: true, + doReplaceAkvVar: false)); + + // Assert + Assert.IsTrue(IsParsed); + Assert.AreEqual("InstrumentationKey=test-key", runtimeConfig.Runtime.Telemetry.ApplicationInsights.ConnectionString, "Connection string should be preserved"); + Assert.AreEqual(expected, runtimeConfig.Runtime.Telemetry.ApplicationInsights.Enabled, "ApplicationInsights enabled value should match expected value"); + } + + /// + /// + /// + /// Value to set in the config to cause error + /// Error message + [TestMethod] + [DataRow("somenonboolean", "Invalid boolean value: somenonboolean. Specify either true or false.", DisplayName = "ApplicationInsights.Enabled invalid value should error")] + public void TestTelemetryApplicationInsightsEnabledShouldError(string configValue, string message) + { + string configJson = @"{ + ""$schema"": ""https://github.com/Azure/data-api-builder/releases/download/vmajor.minor.patch-alpha/dab.draft.schema.json"", + ""data-source"": { + ""database-type"": ""mssql"", + ""connection-string"": ""Server=tcp:127.0.0.1,1433;Persist Security Info=False;Trusted_Connection=True;TrustServerCertificate=True;MultipleActiveResultSets=False;Connection Timeout=5;"" + }, + ""runtime"": { + ""telemetry"": { + ""application-insights"": { + ""enabled"": """ + configValue + @""", + ""connection-string"": ""InstrumentationKey=test-key"" + } + } + }, + ""entities"": { } + }"; + + // Arrange + Mock mockLogger = new(); + + // Act + bool isParsed = RuntimeConfigLoader.TryParseConfig( + configJson, + out RuntimeConfig runtimeConfig, + replacementSettings: new DeserializationVariableReplacementSettings( + azureKeyVaultOptions: null, + doReplaceEnvVar: true, + doReplaceAkvVar: false), + logger: mockLogger.Object); + + // Assert + Assert.IsFalse(isParsed); + Assert.IsNull(runtimeConfig); + + Assert.AreEqual(1, mockLogger.Invocations.Count, "Should raise 1 exception"); + Assert.AreEqual(5, mockLogger.Invocations[0].Arguments.Count, "Log should have 4 arguments"); + var ConfigException = mockLogger.Invocations[0].Arguments[3] as JsonException; + Assert.IsInstanceOfType(ConfigException, typeof(JsonException), "Should have raised a Json Exception"); + Assert.AreEqual(ConfigException.Message, message); + } + /// /// Method to validate that comments are skipped in config file (and are ignored during deserialization). ///