diff --git a/servers/Azure.Mcp.Server/changelog-entries/vcolin7-sql-subscription-resolution.yaml b/servers/Azure.Mcp.Server/changelog-entries/vcolin7-sql-subscription-resolution.yaml new file mode 100644 index 0000000000..367f0bd57e --- /dev/null +++ b/servers/Azure.Mcp.Server/changelog-entries/vcolin7-sql-subscription-resolution.yaml @@ -0,0 +1,4 @@ +changes: + - section: "Bugs Fixed" + description: | + Fixed Azure SQL tools failing with a cryptic ARM error when a subscription display name (instead of a subscription ID) was provided. The `sql server show`, `sql server list`, `sql server create`, and `sql db rename` operations now resolve the subscription through the subscription service. diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs index 0dc73efd80..80216be88f 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs @@ -18,6 +18,7 @@ namespace Azure.Mcp.Tools.Sql.Services; public class SqlService(ISubscriptionService subscriptionService, ITenantService tenantService, ILogger logger) : BaseAzureResourceService(subscriptionService, tenantService), ISqlService { + private readonly ISubscriptionService _subscriptionService = subscriptionService; private readonly ILogger _logger = logger; /// @@ -36,9 +37,7 @@ private async Task GetSqlServerResourceAsync( RetryPolicyOptions? retryPolicy, CancellationToken cancellationToken = default) { - var armClient = await CreateArmClientAsync(null, retryPolicy, null, cancellationToken); - var subscriptionResource = armClient.GetSubscriptionResource( - ResourceManager.Resources.SubscriptionResource.CreateResourceIdentifier(subscription)); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, null, retryPolicy, cancellationToken); var resourceGroupResource = await subscriptionResource.GetResourceGroupAsync(resourceGroup, cancellationToken); return await resourceGroupResource.Value.GetSqlServers().GetAsync(serverName, cancellationToken: cancellationToken); } @@ -317,16 +316,19 @@ public async Task RenameDatabaseAsync( (nameof(subscription), subscription), (nameof(newDatabaseName), newDatabaseName)); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, null, retryPolicy, cancellationToken); + var subscriptionId = subscriptionResource.Data.SubscriptionId; + var armClient = await CreateArmClientAsync(null, retryPolicy, null, cancellationToken); var currentDatabaseId = SqlDatabaseResource.CreateResourceIdentifier( - subscription, + subscriptionId, resourceGroup, serverName, databaseName); var targetDatabaseId = SqlDatabaseResource.CreateResourceIdentifier( - subscription, + subscriptionId, resourceGroup, serverName, newDatabaseName); @@ -632,9 +634,8 @@ public async Task CreateServerAsync( (nameof(administratorLogin), administratorLogin), (nameof(administratorPassword), administratorPassword)); - // Use ARM client directly for create operations - var armClient = await CreateArmClientAsync(null, retryPolicy, null, cancellationToken); - var subscriptionResource = armClient.GetSubscriptionResource(ResourceManager.Resources.SubscriptionResource.CreateResourceIdentifier(subscription)); + // Resolve the subscription (supports both subscription IDs and names) before navigating to the resource group + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, null, retryPolicy, cancellationToken); var resourceGroupResource = await subscriptionResource.GetResourceGroupAsync(resourceGroup, cancellationToken); var serverData = new SqlServerData(location) @@ -730,8 +731,7 @@ public async Task> ListServersAsync( (nameof(resourceGroup), resourceGroup), (nameof(subscription), subscription)); - var armClient = await CreateArmClientAsync(null, retryPolicy, null, cancellationToken); - var subscriptionResource = armClient.GetSubscriptionResource(ResourceManager.Resources.SubscriptionResource.CreateResourceIdentifier(subscription)); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, null, retryPolicy, cancellationToken); ResourceManager.Resources.ResourceGroupResource resourceGroupResource; try diff --git a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs new file mode 100644 index 0000000000..f46adf0d67 --- /dev/null +++ b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Core; +using Azure.Mcp.Core.Services.Azure.Subscription; +using Azure.Mcp.Core.Services.Azure.Tenant; +using Azure.Mcp.Tools.Sql.Services; +using Azure.ResourceManager; +using Microsoft.Extensions.Logging; +using Microsoft.Mcp.Core.Options; +using Microsoft.Mcp.Core.Services.Azure.Authentication; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace Azure.Mcp.Tools.Sql.Tests.Services; + +public class SqlServiceTests +{ + private const string SubscriptionName = "my-subscription-name"; + + // Distinctive message thrown by the mocked subscription service so tests can prove + // the service resolves the subscription via ISubscriptionService (the #449/#453 fix) + // instead of building a SubscriptionResource directly from the raw value. + private const string SubscriptionResolvedMessage = "SqlServiceTests: subscription resolved via ISubscriptionService"; + + private readonly ISubscriptionService _subscriptionService; + private readonly ITenantService _tenantService; + private readonly ILogger _logger; + private readonly SqlService _service; + + public SqlServiceTests() + { + _subscriptionService = Substitute.For(); + _tenantService = Substitute.For(); + _logger = Substitute.For>(); + + var cloudConfig = Substitute.For(); + cloudConfig.CloudType.Returns(AzureCloudConfiguration.AzureCloud.AzurePublicCloud); + cloudConfig.AuthorityHost.Returns(new Uri("https://login.microsoftonline.com")); + cloudConfig.ArmEnvironment.Returns(ArmEnvironment.AzurePublicCloud); + _tenantService.CloudConfiguration.Returns(cloudConfig); + + var credential = Substitute.For(); + _tenantService.GetTokenCredentialAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromResult(credential)); + + _subscriptionService.GetSubscription( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .ThrowsAsync(new InvalidOperationException(SubscriptionResolvedMessage)); + + _service = new SqlService(_subscriptionService, _tenantService, _logger); + } + + [Fact] + public async Task GetServerAsync_ResolvesSubscriptionThroughSubscriptionService() + { + var ex = await Assert.ThrowsAsync(() => + _service.GetServerAsync("server1", "rg", SubscriptionName, null, TestContext.Current.CancellationToken)); + + Assert.Equal(SubscriptionResolvedMessage, ex.Message); + await AssertSubscriptionResolvedAsync(); + } + + [Fact] + public async Task ListServersAsync_ResolvesSubscriptionThroughSubscriptionService() + { + var ex = await Assert.ThrowsAsync(() => + _service.ListServersAsync("rg", SubscriptionName, null, TestContext.Current.CancellationToken)); + + Assert.Equal(SubscriptionResolvedMessage, ex.Message); + await AssertSubscriptionResolvedAsync(); + } + + [Fact] + public async Task CreateServerAsync_ResolvesSubscriptionThroughSubscriptionService() + { + var ex = await Assert.ThrowsAsync(() => + _service.CreateServerAsync( + "server1", + "rg", + SubscriptionName, + "eastus", + "admin", + "P@ssw0rd!", + null, + null, + null, + TestContext.Current.CancellationToken)); + + Assert.Equal(SubscriptionResolvedMessage, ex.Message); + await AssertSubscriptionResolvedAsync(); + } + + [Fact] + public async Task RenameDatabaseAsync_ResolvesSubscriptionThroughSubscriptionService() + { + var ex = await Assert.ThrowsAsync(() => + _service.RenameDatabaseAsync( + "server1", + "olddb", + "newdb", + "rg", + SubscriptionName, + null, + TestContext.Current.CancellationToken)); + + Assert.Equal(SubscriptionResolvedMessage, ex.Message); + await AssertSubscriptionResolvedAsync(); + } + + private Task AssertSubscriptionResolvedAsync() => + _subscriptionService.Received(1).GetSubscription( + SubscriptionName, + Arg.Any(), + Arg.Any(), + Arg.Any()); +} diff --git a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/assets.json b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/assets.json index 6de3f971c2..864cd0afd8 100644 --- a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/assets.json +++ b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/assets.json @@ -2,5 +2,5 @@ "AssetsRepo": "Azure/azure-sdk-assets", "AssetsRepoPrefixPath": "", "TagPrefix": "Azure.Mcp.Tools.Sql.Tests", - "Tag": "Azure.Mcp.Tools.Sql.Tests_a6712a8ecc" + "Tag": "Azure.Mcp.Tools.Sql.Tests_44375821fe" }