From 6fcabfcd2aede222949cf4d92fe2838821a9b938 Mon Sep 17 00:00:00 2001 From: Jesse Squire Date: Wed, 26 Feb 2025 17:07:36 -0800 Subject: [PATCH] [Extensions] Fix ASP.NET Core integration testing (#48426) * [Extensions] Fix ASP.NET Core integration testing The focus of these changes is to fix an assumption in the logic when creating clients from configuration. Currently, if the configuration section has a non-null value, it is assumed to be a connection string. This is not the case for ASP.NET Core test integration due to a bug in the test host which causes the Value property to return an empty string rather than the null returned by the actual configuration system. --- .../Microsoft.Extensions.Azure/CHANGELOG.md | 2 + .../src/Internal/ClientFactory.cs | 8 ++-- .../src/Microsoft.Extensions.Azure.csproj | 2 +- .../tests/ClientFactoryTests.cs | 38 ++++++++++++++++++ .../Microsoft.Extensions.Azure.Tests.csproj | 39 +++++++++++++++++-- .../tests/aspnet-host/AspNetHost.cs | 32 +++++++++++++++ .../tests/aspnet-host/AspNetHostRoot.sln | 0 .../tests/aspnet-host/appsettings.json | 12 ++++++ 8 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHost.cs create mode 100644 sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHostRoot.sln create mode 100644 sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/appsettings.json diff --git a/sdk/extensions/Microsoft.Extensions.Azure/CHANGELOG.md b/sdk/extensions/Microsoft.Extensions.Azure/CHANGELOG.md index 6012112d2467..d812eacfbba5 100644 --- a/sdk/extensions/Microsoft.Extensions.Azure/CHANGELOG.md +++ b/sdk/extensions/Microsoft.Extensions.Azure/CHANGELOG.md @@ -8,6 +8,8 @@ ### Bugs Fixed +- Fixed an issue when creating clients from configuration using the ASP.NET Core integration testing library, `Microsoft.AspNetCore.Mvc.Testing`. Due to a difference in how an section value is represented, logic was interpreting a setting with a child object as an empty connection string value. The child object is now properly parsed and applied for client construction. ([#48368](https://github.com/Azure/azure-sdk-for-net/issues/48368)) + ### Other Changes ## 1.10.0 (2025-02-11) diff --git a/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs b/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs index b5b1221542a1..c330203b737f 100644 --- a/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs +++ b/sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientFactory.cs @@ -30,14 +30,14 @@ public static object CreateClient( { List arguments = new List(); // Handle single values as connection strings - if (configuration is IConfigurationSection section && section.Value != null) + if (configuration is IConfigurationSection section && (!string.IsNullOrEmpty(section.Value))) { var connectionString = section.Value; configuration = new ConfigurationBuilder() - .AddInMemoryCollection(new[] - { + .AddInMemoryCollection( + [ new KeyValuePair(ConnectionStringParameterName, connectionString) - }) + ]) .Build(); } foreach (var constructor in clientType.GetConstructors().OrderByDescending(c => c.GetParameters().Length)) diff --git a/sdk/extensions/Microsoft.Extensions.Azure/src/Microsoft.Extensions.Azure.csproj b/sdk/extensions/Microsoft.Extensions.Azure/src/Microsoft.Extensions.Azure.csproj index 6eb2edc27e73..9677b0459276 100644 --- a/sdk/extensions/Microsoft.Extensions.Azure/src/Microsoft.Extensions.Azure.csproj +++ b/sdk/extensions/Microsoft.Extensions.Azure/src/Microsoft.Extensions.Azure.csproj @@ -25,7 +25,7 @@ - + diff --git a/sdk/extensions/Microsoft.Extensions.Azure/tests/ClientFactoryTests.cs b/sdk/extensions/Microsoft.Extensions.Azure/tests/ClientFactoryTests.cs index 1f38862eb0d3..a5a165bba9ef 100644 --- a/sdk/extensions/Microsoft.Extensions.Azure/tests/ClientFactoryTests.cs +++ b/sdk/extensions/Microsoft.Extensions.Azure/tests/ClientFactoryTests.cs @@ -12,6 +12,13 @@ using Microsoft.Extensions.Configuration; using NUnit.Framework; +#if NET8_0_OR_GREATER +using System.Threading.Tasks; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.AspNetCore.TestHost; +#endif + namespace Azure.Core.Extensions.Tests { public class ClientFactoryTests @@ -626,6 +633,37 @@ public void CanUseAzureSasCredential() Assert.AreEqual("key", client.AzureSasCredential.Signature); } +#if NET8_0_OR_GREATER + [Test] + public async Task AllowsAspNetCoreIntegrationTestHostConfiguration() + { + var expectedKeyVaultUriValue = "https://fake.vault.azure.net/"; + + // When configuration is set using the test host, the behavior of IConfigurationSection + // changes and the Value property is not null for a complex object. Instead it returns an + // empty string, which we don't want to treat as a connection string. + // + // This is a bug in the configuration system, but there's no commitment for when it will + // be fixed. See: https://github.com/dotnet/aspnetcore/issues/37680 + // + var factory = new WebApplicationFactory() + .WithWebHostBuilder(builder => + { + builder.UseContentRoot("aspnet-host"); + + builder.UseConfiguration( + GetConfiguration( + new KeyValuePair("KeyVault:VaultUri", expectedKeyVaultUriValue))); + }); + + var client = factory.CreateClient(); + var response = await client.GetAsync("/keyvault"); + var keyVaultUriValue = await response.Content.ReadAsStringAsync(); + + Assert.AreEqual(expectedKeyVaultUriValue, keyVaultUriValue); + } +#endif + private IConfiguration GetConfiguration(params KeyValuePair[] items) { return new ConfigurationBuilder().AddInMemoryCollection(items).Build(); diff --git a/sdk/extensions/Microsoft.Extensions.Azure/tests/Microsoft.Extensions.Azure.Tests.csproj b/sdk/extensions/Microsoft.Extensions.Azure/tests/Microsoft.Extensions.Azure.Tests.csproj index 39136ce31ab4..878c4a8f501a 100644 --- a/sdk/extensions/Microsoft.Extensions.Azure/tests/Microsoft.Extensions.Azure.Tests.csproj +++ b/sdk/extensions/Microsoft.Extensions.Azure/tests/Microsoft.Extensions.Azure.Tests.csproj @@ -2,19 +2,50 @@ $(RequiredTargetFrameworks) + + + - + + - - - + + + + + + + + + + + + + + + false + + + + + + + + + + Always + + \ No newline at end of file diff --git a/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHost.cs b/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHost.cs new file mode 100644 index 000000000000..4af0390d194f --- /dev/null +++ b/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHost.cs @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +#if NET8_0_OR_GREATER + +using Azure.Security.KeyVault.Secrets; +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.Azure; +using Microsoft.Extensions.DependencyInjection; + +namespace Azure.Core.Extensions.Tests; + +public class AspNetHost +{ + public static void Main(string[] args) + { + var builder = WebApplication.CreateBuilder(args); + builder.Environment.ContentRootPath = "aspnet-host"; + + builder.Services.AddAzureClients(clientBuilder => + { + clientBuilder.AddSecretClient(builder.Configuration.GetSection("KeyVault")); + }); + + var app = builder.Build(); + var secretClient = app.Services.GetRequiredService(); + + app.MapGet("/keyvault", () => secretClient.VaultUri.AbsoluteUri); + app.Run(); + } +} +#endif \ No newline at end of file diff --git a/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHostRoot.sln b/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/AspNetHostRoot.sln new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/appsettings.json b/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/appsettings.json new file mode 100644 index 000000000000..0ee3a669196e --- /dev/null +++ b/sdk/extensions/Microsoft.Extensions.Azure/tests/aspnet-host/appsettings.json @@ -0,0 +1,12 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + }, + "AllowedHosts": "*", + "KeyVault": { + "VaultUri": "" + } +}