From a1dcc585f84c9fda8f8c9eda0739424281c36519 Mon Sep 17 00:00:00 2001 From: Victor Colin Amador Date: Mon, 8 Jun 2026 15:28:17 -0700 Subject: [PATCH 1/4] Fix SQL list databases and elastic pools to filter by server (#448, #452) ListDatabasesAsync and GetElasticPoolsAsync ignored serverName and returned all resources in the resource group. Scope the Azure Resource Graph query to the requested server via an id-contains additionalFilter, extracted into a testable BuildServerResourceFilter helper, and add unit tests covering the filter and KQL escaping. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilot-fix-sql-server-filter.yaml | 3 ++ .../src/Services/SqlService.cs | 13 ++++++ .../Services/SqlServiceTests.cs | 40 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml create mode 100644 tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs diff --git a/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml b/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml new file mode 100644 index 0000000000..1cb912b374 --- /dev/null +++ b/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml @@ -0,0 +1,3 @@ +changes: + - section: "Bugs Fixed" + description: "Fixed the SQL `db get` (list databases) and `elastic-pool list` tools returning resources from every server in the resource group instead of only the requested server. The Azure Resource Graph query is now scoped to the specified server." diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs index 0dc73efd80..27ecd08287 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs @@ -20,6 +20,17 @@ public class SqlService(ISubscriptionService subscriptionService, ITenantService { private readonly ILogger _logger = logger; + /// + /// Builds a KQL filter that scopes an Azure Resource Graph query to resources belonging to a + /// specific SQL server. SQL child resources (databases, elastic pools) embed the parent server + /// name in their resource id as "/servers/{serverName}/", so filtering on the id restricts the + /// results to the requested server instead of every server in the resource group. + /// + /// The name of the SQL server to scope results to. + /// A KQL filter expression suitable for the additionalFilter parameter. + internal static string BuildServerResourceFilter(string serverName) => + $"id contains '/servers/{EscapeKqlString(serverName)}/'"; + /// /// Helper method to navigate the Azure resource hierarchy and retrieve a SQL Server resource. /// @@ -368,6 +379,7 @@ public async Task> ListDatabasesAsync( subscription, retryPolicy, ConvertToSqlDatabaseModel, + additionalFilter: BuildServerResourceFilter(serverName), cancellationToken: cancellationToken); } @@ -442,6 +454,7 @@ public async Task> GetElasticPoolsAsync( subscription, retryPolicy, ConvertToSqlElasticPoolModel, + additionalFilter: BuildServerResourceFilter(serverName), cancellationToken: cancellationToken); } 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..c309e9994e --- /dev/null +++ b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Mcp.Tools.Sql.Services; +using Xunit; + +namespace Azure.Mcp.Tools.Sql.Tests.Services; + +public class SqlServiceTests +{ + [Fact] + public void BuildServerResourceFilter_ScopesQueryToServer() + { + // Ensures list operations (databases, elastic pools) are filtered to a single SQL server + // rather than returning every resource in the resource group (issues #448 and #452). + var filter = SqlService.BuildServerResourceFilter("server1"); + + Assert.Equal("id contains '/servers/server1/'", filter); + } + + [Theory] + [InlineData("my-server", "id contains '/servers/my-server/'")] + [InlineData("o'brien", "id contains '/servers/o''brien/'")] + [InlineData("back\\slash", "id contains '/servers/back\\\\slash/'")] + public void BuildServerResourceFilter_EscapesServerNameForKql(string serverName, string expected) + { + var filter = SqlService.BuildServerResourceFilter(serverName); + + Assert.Equal(expected, filter); + } + + [Fact] + public void BuildServerResourceFilter_DoesNotContainPipeOperator() + { + // The base resource query rejects additional filters containing '|' to prevent KQL injection. + var filter = SqlService.BuildServerResourceFilter("server1"); + + Assert.DoesNotContain('|', filter); + } +} From 0444fc168334a2dfeaae1bb9df2292b09a8f67c4 Mon Sep 17 00:00:00 2001 From: Victor Colin Amador Date: Mon, 8 Jun 2026 22:54:34 -0700 Subject: [PATCH 2/4] Switch SQL list databases/elastic pools to ARM SDK (#448, #452) Align ListDatabasesAsync and GetElasticPoolsAsync with the sibling list methods (GetEntraAdministratorsAsync, ListFirewallRulesAsync, ListServersAsync) by using the server-scoped ARM SDK collections instead of an Azure Resource Graph 'id contains' filter. Server scoping is now structural (the server is part of the request path) rather than a KQL predicate, and the SDK auto-pages the full set so the ResourceQueryResults truncation model is no longer needed. - ISqlService/SqlService: return List; iterate GetSqlDatabases()/GetElasticPools().GetAllAsync(); add SDK-based ConvertToSqlElasticPoolModel(ElasticPoolResource). - Commands: consume List; drop AreResultsTruncated from result records (matches FirewallRuleListResult). - Remove now-orphaned elastic-pool JSON shadow models and their serialization registrations; drop obsolete BuildServerResourceFilter helper and its unit test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../copilot-fix-sql-server-filter.yaml | 2 +- .../Commands/Database/DatabaseGetCommand.cs | 8 +- .../ElasticPool/ElasticPoolListCommand.cs | 4 +- .../src/Commands/SqlJsonContext.cs | 3 - .../src/Services/ISqlService.cs | 5 +- .../src/Services/Models/SqlElasticPoolData.cs | 38 ------ .../SqlElasticPoolPerDatabaseSettings.cs | 14 --- .../Models/SqlElasticPoolProperties.cs | 29 ----- .../src/Services/SqlService.cs | 108 ++++++++++-------- .../Database/DatabaseGetCommandTests.cs | 5 +- .../ElasticPoolListCommandTests.cs | 11 +- .../Services/SqlServiceTests.cs | 40 ------- 12 files changed, 74 insertions(+), 193 deletions(-) delete mode 100644 tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolData.cs delete mode 100644 tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolPerDatabaseSettings.cs delete mode 100644 tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolProperties.cs delete mode 100644 tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs diff --git a/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml b/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml index 1cb912b374..34455d1f0a 100644 --- a/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml +++ b/servers/Azure.Mcp.Server/changelog-entries/copilot-fix-sql-server-filter.yaml @@ -1,3 +1,3 @@ changes: - section: "Bugs Fixed" - description: "Fixed the SQL `db get` (list databases) and `elastic-pool list` tools returning resources from every server in the resource group instead of only the requested server. The Azure Resource Graph query is now scoped to the specified server." + description: "Fixed the SQL `db get` (list databases) and `elastic-pool list` tools returning resources from every server in the resource group instead of only the requested server. The queries are now scoped to the specified server." diff --git a/tools/Azure.Mcp.Tools.Sql/src/Commands/Database/DatabaseGetCommand.cs b/tools/Azure.Mcp.Tools.Sql/src/Commands/Database/DatabaseGetCommand.cs index 0cfccc479c..d55ee35c90 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Commands/Database/DatabaseGetCommand.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Commands/Database/DatabaseGetCommand.cs @@ -71,12 +71,12 @@ public override async Task ExecuteAsync(CommandContext context, cancellationToken); context.Response.Results = ResponseResult.Create( - new([database], false), + new([database]), SqlJsonContext.Default.DatabaseGetListResult); } else { - var result = await _sqlService.ListDatabasesAsync( + var databases = await _sqlService.ListDatabasesAsync( options.Server!, options.ResourceGroup!, options.Subscription!, @@ -84,7 +84,7 @@ public override async Task ExecuteAsync(CommandContext context, cancellationToken); context.Response.Results = ResponseResult.Create( - new(result?.Results ?? [], result?.AreResultsTruncated ?? false), + new(databases ?? []), SqlJsonContext.Default.DatabaseGetListResult); } } @@ -109,5 +109,5 @@ public override async Task ExecuteAsync(CommandContext context, _ => base.GetErrorMessage(ex) }; - internal record DatabaseGetListResult(List Databases, bool AreResultsTruncated); + internal record DatabaseGetListResult(List Databases); } diff --git a/tools/Azure.Mcp.Tools.Sql/src/Commands/ElasticPool/ElasticPoolListCommand.cs b/tools/Azure.Mcp.Tools.Sql/src/Commands/ElasticPool/ElasticPoolListCommand.cs index 1dd9d99fe1..3c5082cb1d 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Commands/ElasticPool/ElasticPoolListCommand.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Commands/ElasticPool/ElasticPoolListCommand.cs @@ -52,7 +52,7 @@ public override async Task ExecuteAsync(CommandContext context, options.RetryPolicy, cancellationToken); - context.Response.Results = ResponseResult.Create(new(elasticPools?.Results ?? [], elasticPools?.AreResultsTruncated ?? false), SqlJsonContext.Default.ElasticPoolListResult); + context.Response.Results = ResponseResult.Create(new(elasticPools ?? []), SqlJsonContext.Default.ElasticPoolListResult); } catch (Exception ex) { @@ -75,5 +75,5 @@ public override async Task ExecuteAsync(CommandContext context, _ => base.GetErrorMessage(ex) }; - internal record ElasticPoolListResult(List ElasticPools, bool AreResultsTruncated); + internal record ElasticPoolListResult(List ElasticPools); } diff --git a/tools/Azure.Mcp.Tools.Sql/src/Commands/SqlJsonContext.cs b/tools/Azure.Mcp.Tools.Sql/src/Commands/SqlJsonContext.cs index ea02b87fe1..137125513f 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Commands/SqlJsonContext.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Commands/SqlJsonContext.cs @@ -36,9 +36,6 @@ namespace Azure.Mcp.Tools.Sql.Commands; [JsonSerializable(typeof(SqlDatabaseData))] [JsonSerializable(typeof(SqlDatabaseProperties))] [JsonSerializable(typeof(SqlServerAadAdministratorData))] -[JsonSerializable(typeof(SqlElasticPoolData))] -[JsonSerializable(typeof(SqlElasticPoolProperties))] -[JsonSerializable(typeof(SqlElasticPoolPerDatabaseSettings))] [JsonSerializable(typeof(SqlFirewallRuleData))] [JsonSourceGenerationOptions( PropertyNamingPolicy = JsonKnownNamingPolicy.CamelCase, diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/ISqlService.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/ISqlService.cs index b49deb92f9..fe5af8975a 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/ISqlService.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Services/ISqlService.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using Azure.Mcp.Core.Services.Azure; using Azure.Mcp.Tools.Sql.Models; using Microsoft.Mcp.Core.Options; @@ -125,7 +124,7 @@ Task RenameDatabaseAsync( /// Optional retry policy options /// Cancellation token /// A list of SQL databases - Task> ListDatabasesAsync( + Task> ListDatabasesAsync( string serverName, string resourceGroup, string subscription, @@ -157,7 +156,7 @@ Task> GetEntraAdministratorsAsync( /// Optional retry policy options /// Cancellation token /// A list of SQL elastic pools - Task> GetElasticPoolsAsync( + Task> GetElasticPoolsAsync( string serverName, string resourceGroup, string subscription, diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolData.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolData.cs deleted file mode 100644 index e1cc12b1c6..0000000000 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolData.cs +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System.Text.Json; -using System.Text.Json.Serialization; -using Azure.Mcp.Tools.Sql.Commands; - -namespace Azure.Mcp.Tools.Sql.Services.Models -{ - /// - /// A class representing the ElasticPool data model. - /// An elastic pool. - /// - internal sealed class SqlElasticPoolData - { - /// The resource ID for the resource. - [JsonPropertyName("id")] - public string? ResourceId { get; set; } - /// The type of the resource. - [JsonPropertyName("type")] - public string? ResourceType { get; set; } - /// The name of the resource. - [JsonPropertyName("name")] - public string? ResourceName { get; set; } - /// The location of the resource. - public string? Location { get; set; } - /// The database SKU. - public SqlSku? Sku { get; set; } - /// The properties of elastic pool. - public SqlElasticPoolProperties? Properties { get; set; } - - // Read the JSON response content and create a model instance from it. - public static SqlElasticPoolData? FromJson(JsonElement source) - { - return JsonSerializer.Deserialize(source, SqlJsonContext.Default.SqlElasticPoolData); - } - } -} diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolPerDatabaseSettings.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolPerDatabaseSettings.cs deleted file mode 100644 index 9ccb6de1ce..0000000000 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolPerDatabaseSettings.cs +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -namespace Azure.Mcp.Tools.Sql.Services.Models -{ - /// Per database settings of an elastic pool. - internal sealed class SqlElasticPoolPerDatabaseSettings - { - /// The minimum capacity all databases are guaranteed. - public double? MinCapacity { get; set; } - /// The maximum capacity any one database can consume. - public double? MaxCapacity { get; set; } - } -} diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolProperties.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolProperties.cs deleted file mode 100644 index d071109820..0000000000 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/Models/SqlElasticPoolProperties.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -using System.Text.Json.Serialization; - -namespace Azure.Mcp.Tools.Sql.Services.Models -{ - /// - /// A class representing the ElasticPool properties model. - /// An elastic pool properties. - /// - internal sealed class SqlElasticPoolProperties - { - /// The state of the elastic pool. - public string? State { get; set; } - /// The creation date of the elastic pool (ISO8601 format). - [JsonPropertyName("creationDate")] - public DateTimeOffset? CreatedOn { get; set; } - /// The storage limit for the database elastic pool in bytes. - public long? MaxSizeBytes { get; set; } - /// The per database settings for the elastic pool. - public SqlElasticPoolPerDatabaseSettings? PerDatabaseSettings { get; set; } - /// Whether or not this elastic pool is zone redundant, which means the replicas of this elastic pool will be spread across multiple availability zones. - [JsonPropertyName("zoneRedundant")] - public bool? IsZoneRedundant { get; set; } - /// The license type to apply for this elastic pool. - public string? LicenseType { get; set; } - } -} diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs index 27ecd08287..ddfef049fa 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs @@ -20,17 +20,6 @@ public class SqlService(ISubscriptionService subscriptionService, ITenantService { private readonly ILogger _logger = logger; - /// - /// Builds a KQL filter that scopes an Azure Resource Graph query to resources belonging to a - /// specific SQL server. SQL child resources (databases, elastic pools) embed the parent server - /// name in their resource id as "/servers/{serverName}/", so filtering on the id restricts the - /// results to the requested server instead of every server in the resource group. - /// - /// The name of the SQL server to scope results to. - /// A KQL filter expression suitable for the additionalFilter parameter. - internal static string BuildServerResourceFilter(string serverName) => - $"id contains '/servers/{EscapeKqlString(serverName)}/'"; - /// /// Helper method to navigate the Azure resource hierarchy and retrieve a SQL Server resource. /// @@ -366,21 +355,31 @@ public async Task RenameDatabaseAsync( /// Token to observe for cancellation requests /// A list of SQL databases on the specified server /// Thrown when required parameters are null or empty - public async Task> ListDatabasesAsync( + public async Task> ListDatabasesAsync( string serverName, string resourceGroup, string subscription, RetryPolicyOptions? retryPolicy, CancellationToken cancellationToken = default) { - return await ExecuteResourceQueryAsync( - "Microsoft.Sql/servers/databases", - resourceGroup, - subscription, - retryPolicy, - ConvertToSqlDatabaseModel, - additionalFilter: BuildServerResourceFilter(serverName), - cancellationToken: cancellationToken); + ValidateRequiredParameters( + (nameof(serverName), serverName), + (nameof(resourceGroup), resourceGroup), + (nameof(subscription), subscription)); + + var sqlServerResource = await GetSqlServerResourceAsync(serverName, resourceGroup, subscription, retryPolicy, cancellationToken); + + var databases = new List(); + await foreach (var database in sqlServerResource.GetSqlDatabases().GetAllAsync(cancellationToken: cancellationToken)) + { + databases.Add(ConvertToSqlDatabaseModel(database)); + } + + _logger.LogInformation( + "Successfully listed SQL databases. Server: {Server}, ResourceGroup: {ResourceGroup}, Count: {Count}", + serverName, resourceGroup, databases.Count); + + return databases; } /// @@ -441,21 +440,31 @@ public async Task> GetEntraAdministratorsAsync /// Token to observe for cancellation requests /// A list of elastic pools configured on the SQL server /// Thrown when required parameters are null or empty - public async Task> GetElasticPoolsAsync( + public async Task> GetElasticPoolsAsync( string serverName, string resourceGroup, string subscription, RetryPolicyOptions? retryPolicy, CancellationToken cancellationToken = default) { - return await ExecuteResourceQueryAsync( - "Microsoft.Sql/servers/elasticPools", - resourceGroup, - subscription, - retryPolicy, - ConvertToSqlElasticPoolModel, - additionalFilter: BuildServerResourceFilter(serverName), - cancellationToken: cancellationToken); + ValidateRequiredParameters( + (nameof(serverName), serverName), + (nameof(resourceGroup), resourceGroup), + (nameof(subscription), subscription)); + + var sqlServerResource = await GetSqlServerResourceAsync(serverName, resourceGroup, subscription, retryPolicy, cancellationToken); + + var elasticPools = new List(); + await foreach (var elasticPool in sqlServerResource.GetElasticPools().GetAllAsync(cancellationToken: cancellationToken)) + { + elasticPools.Add(ConvertToSqlElasticPoolModel(elasticPool)); + } + + _logger.LogInformation( + "Successfully listed SQL elastic pools. Server: {Server}, ResourceGroup: {ResourceGroup}, Count: {Count}", + serverName, resourceGroup, elasticPools.Count); + + return elasticPools; } /// @@ -930,32 +939,31 @@ private static SqlServer ConvertToSqlServerModel(SqlServerResource serverResourc Tags: tags.Count > 0 ? tags : null); } - private static SqlElasticPool ConvertToSqlElasticPoolModel(JsonElement item) + private static SqlElasticPool ConvertToSqlElasticPoolModel(ElasticPoolResource elasticPoolResource) { - SqlElasticPoolData? elasticPool = SqlElasticPoolData.FromJson(item) - ?? throw new InvalidOperationException("Failed to parse SQL elastic pool data"); + var data = elasticPoolResource.Data; return new( - Name: elasticPool.ResourceName ?? "Unknown", - Id: elasticPool.ResourceId ?? "Unknown", - Type: elasticPool.ResourceType ?? "Unknown", - Location: elasticPool.Location, - Sku: elasticPool.Sku != null ? new( - Name: elasticPool.Sku.Name, - Tier: elasticPool.Sku.Tier, - Capacity: elasticPool.Sku.Capacity, - Family: elasticPool.Sku.Family, - Size: elasticPool.Sku.Size + Name: data.Name, + Id: data.Id.ToString(), + Type: data.ResourceType.ToString(), + Location: data.Location.ToString(), + Sku: data.Sku != null ? new( + Name: data.Sku.Name, + Tier: data.Sku.Tier, + Capacity: data.Sku.Capacity, + Family: data.Sku.Family, + Size: data.Sku.Size ) : null, - State: elasticPool.Properties?.State, - CreationDate: elasticPool.Properties?.CreatedOn, - MaxSizeBytes: elasticPool.Properties?.MaxSizeBytes, - PerDatabaseSettings: elasticPool.Properties?.PerDatabaseSettings != null ? new( - MinCapacity: elasticPool.Properties.PerDatabaseSettings.MinCapacity, - MaxCapacity: elasticPool.Properties.PerDatabaseSettings.MaxCapacity + State: data.State?.ToString(), + CreationDate: data.CreatedOn, + MaxSizeBytes: data.MaxSizeBytes, + PerDatabaseSettings: data.PerDatabaseSettings != null ? new( + MinCapacity: data.PerDatabaseSettings.MinCapacity, + MaxCapacity: data.PerDatabaseSettings.MaxCapacity ) : null, - ZoneRedundant: elasticPool.Properties?.IsZoneRedundant, - LicenseType: elasticPool.Properties?.LicenseType, + ZoneRedundant: data.IsZoneRedundant, + LicenseType: data.LicenseType?.ToString(), DatabaseDtuMin: null, // DTU properties not available in current SDK DatabaseDtuMax: null, Dtu: null, diff --git a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Database/DatabaseGetCommandTests.cs b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Database/DatabaseGetCommandTests.cs index 0c5cb0a323..5450b1cc2b 100644 --- a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Database/DatabaseGetCommandTests.cs +++ b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Database/DatabaseGetCommandTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System.Net; -using Azure.Mcp.Core.Services.Azure; using Azure.Mcp.Tools.Sql.Commands.Database; using Azure.Mcp.Tools.Sql.Models; using Azure.Mcp.Tools.Sql.Services; @@ -59,7 +58,7 @@ public async Task ExecuteAsync_WithDatabaseName_ReturnsSingleDatabase() public async Task ExecuteAsync_WithoutDatabaseName_ReturnsAllDatabases() { // Arrange - var mockDatabases = new ResourceQueryResults([CreateMockDatabase("db1"), CreateMockDatabase("db2")], false); + var mockDatabases = new List { CreateMockDatabase("db1"), CreateMockDatabase("db2") }; Service.ListDatabasesAsync( Arg.Is("server1"), @@ -171,7 +170,7 @@ public async Task ExecuteAsync_ValidatesRequiredParameters(string commandArgs, b { Service .ListDatabasesAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(new ResourceQueryResults([], false)); + .Returns(new List()); Service .GetDatabaseAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) .Returns(CreateMockDatabase("db1")); diff --git a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/ElasticPool/ElasticPoolListCommandTests.cs b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/ElasticPool/ElasticPoolListCommandTests.cs index 65578d7d14..c37523ca62 100644 --- a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/ElasticPool/ElasticPoolListCommandTests.cs +++ b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/ElasticPool/ElasticPoolListCommandTests.cs @@ -2,7 +2,6 @@ // Licensed under the MIT License. using System.Net; -using Azure.Mcp.Core.Services.Azure; using Azure.Mcp.Tools.Sql.Commands.ElasticPool; using Azure.Mcp.Tools.Sql.Models; using Azure.Mcp.Tools.Sql.Services; @@ -28,8 +27,8 @@ public void Constructor_InitializesCommandCorrectly() public async Task ExecuteAsync_WithValidParameters_ReturnsElasticPools() { // Arrange - var mockElasticPools = new ResourceQueryResults( - [ + var mockElasticPools = new List + { new( Name: "pool1", Id: "/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Sql/servers/server1/elasticPools/pool1", @@ -47,7 +46,7 @@ public async Task ExecuteAsync_WithValidParameters_ReturnsElasticPools() Dtu: 100, StorageMB: 5120 ) - ], false); + }; Service.GetElasticPoolsAsync( Arg.Is("server1"), @@ -74,7 +73,7 @@ public async Task ExecuteAsync_WithValidParameters_ReturnsElasticPools() public async Task ExecuteAsync_WithEmptyList_ReturnsEmptyResults() { // Arrange - var mockElasticPools = new ResourceQueryResults([], false); + var mockElasticPools = new List(); Service.GetElasticPoolsAsync( Arg.Is("server1"), @@ -186,7 +185,7 @@ public async Task ExecuteAsync_ValidatesRequiredParameters(string args, bool sho Arg.Any(), Arg.Any(), Arg.Any()) - .Returns(new ResourceQueryResults([], false)); + .Returns(new List()); } // Act 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 deleted file mode 100644 index c309e9994e..0000000000 --- a/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Azure.Mcp.Tools.Sql.Services; -using Xunit; - -namespace Azure.Mcp.Tools.Sql.Tests.Services; - -public class SqlServiceTests -{ - [Fact] - public void BuildServerResourceFilter_ScopesQueryToServer() - { - // Ensures list operations (databases, elastic pools) are filtered to a single SQL server - // rather than returning every resource in the resource group (issues #448 and #452). - var filter = SqlService.BuildServerResourceFilter("server1"); - - Assert.Equal("id contains '/servers/server1/'", filter); - } - - [Theory] - [InlineData("my-server", "id contains '/servers/my-server/'")] - [InlineData("o'brien", "id contains '/servers/o''brien/'")] - [InlineData("back\\slash", "id contains '/servers/back\\\\slash/'")] - public void BuildServerResourceFilter_EscapesServerNameForKql(string serverName, string expected) - { - var filter = SqlService.BuildServerResourceFilter(serverName); - - Assert.Equal(expected, filter); - } - - [Fact] - public void BuildServerResourceFilter_DoesNotContainPipeOperator() - { - // The base resource query rejects additional filters containing '|' to prevent KQL injection. - var filter = SqlService.BuildServerResourceFilter("server1"); - - Assert.DoesNotContain('|', filter); - } -} From 668be770ea112921f385dda2724fdc02d1ded9c1 Mon Sep 17 00:00:00 2001 From: Victor Colin Amador Date: Thu, 11 Jun 2026 19:48:39 -0700 Subject: [PATCH 3/4] Re-record SQL list databases/elastic pools tests after ARM SDK switch (#448, #452) The switch from Azure Resource Graph to the typed ARM SDK changed the outgoing REST requests for ListDatabasesAsync and GetElasticPoolsAsync, invalidating the existing SqlCommandTests recordings. Re-recorded the SqlCommandTests session records and updated assets.json to the new tag (Azure.Mcp.Tools.Sql.Tests_50222315f7). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../tests/Azure.Mcp.Tools.Sql.Tests/assets.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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..c8202d42fd 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_50222315f7" } From 754015eb698e30821763b74ffd2d4b903cc66b55 Mon Sep 17 00:00:00 2001 From: Victor Colin Amador Date: Mon, 15 Jun 2026 18:29:23 -0600 Subject: [PATCH 4/4] Resolve subscription name or ID for SQL list databases and elastic pools (#448, #452) The ARM SDK refactor of ListDatabasesAsync and GetElasticPoolsAsync routed the subscription through SubscriptionResource.CreateResourceIdentifier, which only accepts a subscription GUID. This regressed the prior Resource Graph behavior where a subscription name also worked, as flagged by Copilot review on PR #2865. Scope the fix narrowly to just these two list methods by resolving the subscription via ISubscriptionService before navigating the ARM hierarchy. The shared GetSqlServerResourceAsync helper and the server/firewall/entra-admin/create/rename paths are intentionally left unchanged to avoid overlapping with PR #2861. The new ResolveSubscriptionIdAsync helper skips the extra ARM request when the value is already a subscription ID, so existing recordings remain valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/Services/SqlService.cs | 26 ++++- .../Services/SqlServiceTests.cs | 103 ++++++++++++++++++ 2 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs diff --git a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs index ddfef049fa..19a8e8546a 100644 --- a/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs +++ b/tools/Azure.Mcp.Tools.Sql/src/Services/SqlService.cs @@ -18,8 +18,28 @@ 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; + /// + /// Resolves a subscription name or ID to a subscription ID. When the value is already a + /// subscription ID no additional ARM request is made, preserving the existing network + /// behavior for ID-based callers. + /// + /// The subscription ID or name + /// Optional retry policy configuration + /// Token to observe for cancellation requests + /// The resolved subscription ID + private async Task ResolveSubscriptionIdAsync( + string subscription, + RetryPolicyOptions? retryPolicy, + CancellationToken cancellationToken) + { + return _subscriptionService.IsSubscriptionId(subscription) + ? subscription + : await _subscriptionService.GetSubscriptionIdByName(subscription, null, retryPolicy, cancellationToken); + } + /// /// Helper method to navigate the Azure resource hierarchy and retrieve a SQL Server resource. /// @@ -367,7 +387,8 @@ public async Task> ListDatabasesAsync( (nameof(resourceGroup), resourceGroup), (nameof(subscription), subscription)); - var sqlServerResource = await GetSqlServerResourceAsync(serverName, resourceGroup, subscription, retryPolicy, cancellationToken); + var subscriptionId = await ResolveSubscriptionIdAsync(subscription, retryPolicy, cancellationToken); + var sqlServerResource = await GetSqlServerResourceAsync(serverName, resourceGroup, subscriptionId, retryPolicy, cancellationToken); var databases = new List(); await foreach (var database in sqlServerResource.GetSqlDatabases().GetAllAsync(cancellationToken: cancellationToken)) @@ -452,7 +473,8 @@ public async Task> GetElasticPoolsAsync( (nameof(resourceGroup), resourceGroup), (nameof(subscription), subscription)); - var sqlServerResource = await GetSqlServerResourceAsync(serverName, resourceGroup, subscription, retryPolicy, cancellationToken); + var subscriptionId = await ResolveSubscriptionIdAsync(subscription, retryPolicy, cancellationToken); + var sqlServerResource = await GetSqlServerResourceAsync(serverName, resourceGroup, subscriptionId, retryPolicy, cancellationToken); var elasticPools = new List(); await foreach (var elasticPool in sqlServerResource.GetElasticPools().GetAllAsync(cancellationToken: cancellationToken)) 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..7ab7490340 --- /dev/null +++ b/tools/Azure.Mcp.Tools.Sql/tests/Azure.Mcp.Tools.Sql.Tests/Services/SqlServiceTests.cs @@ -0,0 +1,103 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using Azure.Mcp.Core.Services.Azure.Subscription; +using Azure.Mcp.Core.Services.Azure.Tenant; +using Azure.Mcp.Tools.Sql.Services; +using Microsoft.Extensions.Logging; +using Microsoft.Mcp.Core.Options; +using NSubstitute; +using NSubstitute.ExceptionExtensions; +using Xunit; + +namespace Azure.Mcp.Tools.Sql.Tests.Services; + +public class SqlServiceTests +{ + private const string SubscriptionName = "my-subscription"; + private const string SubscriptionId = "12345678-1234-1234-1234-123456789012"; + private const string ResolveSentinel = "SqlServiceTests: subscription name resolved via ISubscriptionService"; + private const string ServerName = "server1"; + private const string ResourceGroup = "rg1"; + + 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>(); + _service = new SqlService(_subscriptionService, _tenantService, _logger); + } + + [Fact] + public async Task ListDatabasesAsync_WithSubscriptionName_ResolvesNameToId() + { + _subscriptionService.IsSubscriptionId(SubscriptionName).Returns(false); + _subscriptionService.GetSubscriptionIdByName(SubscriptionName, Arg.Any(), Arg.Any(), Arg.Any()) + .ThrowsAsync(new InvalidOperationException(ResolveSentinel)); + + var exception = await Assert.ThrowsAsync( + () => _service.ListDatabasesAsync(ServerName, ResourceGroup, SubscriptionName, null, TestContext.Current.CancellationToken)); + + Assert.Equal(ResolveSentinel, exception.Message); + await _subscriptionService.Received(1).GetSubscriptionIdByName( + SubscriptionName, Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GetElasticPoolsAsync_WithSubscriptionName_ResolvesNameToId() + { + _subscriptionService.IsSubscriptionId(SubscriptionName).Returns(false); + _subscriptionService.GetSubscriptionIdByName(SubscriptionName, Arg.Any(), Arg.Any(), Arg.Any()) + .ThrowsAsync(new InvalidOperationException(ResolveSentinel)); + + var exception = await Assert.ThrowsAsync( + () => _service.GetElasticPoolsAsync(ServerName, ResourceGroup, SubscriptionName, null, TestContext.Current.CancellationToken)); + + Assert.Equal(ResolveSentinel, exception.Message); + await _subscriptionService.Received(1).GetSubscriptionIdByName( + SubscriptionName, Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task ListDatabasesAsync_WithSubscriptionId_SkipsNameLookup() + { + _subscriptionService.IsSubscriptionId(SubscriptionId).Returns(true); + var canceled = new CancellationToken(canceled: true); + + try + { + await _service.ListDatabasesAsync(ServerName, ResourceGroup, SubscriptionId, null, canceled); + } + catch + { + // The ARM hierarchy call is expected to fail/cancel; we only assert resolution behavior. + } + + await _subscriptionService.DidNotReceive().GetSubscriptionIdByName( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } + + [Fact] + public async Task GetElasticPoolsAsync_WithSubscriptionId_SkipsNameLookup() + { + _subscriptionService.IsSubscriptionId(SubscriptionId).Returns(true); + var canceled = new CancellationToken(canceled: true); + + try + { + await _service.GetElasticPoolsAsync(ServerName, ResourceGroup, SubscriptionId, null, canceled); + } + catch + { + // The ARM hierarchy call is expected to fail/cancel; we only assert resolution behavior. + } + + await _subscriptionService.DidNotReceive().GetSubscriptionIdByName( + Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()); + } +}