Accept tenant when calling Azure Resource Graph queries#1945
Accept tenant when calling Azure Resource Graph queries#1945alzimmermsft wants to merge 1 commit intomicrosoft:mainfrom
Conversation
github-actions
bot
added
tools-ACR
tools-AppConfig
alzimmermsft
requested review from
a team,
JonathanCrd,
conniey,
danield137,
jongio,
prvavill,
vurhanau,
weng5e and
xiangyan99
as code owners
alzimmermsft
requested review from
chidozieononiwu,
Copilot,
g2vinay and
vukelich
There was a problem hiding this comment.
Pull request overview
This PR extends Azure Resource Graph (ARG) query execution to accept an optional tenant, enabling callers to scope queries by both tenant and subscription.
Changes:
- Adds an optional
tenantparameter toBaseAzureResourceService.ExecuteResourceQueryAsyncand uses it when resolving the subscription/tenant context. - Refactors single-resource ARG querying to reuse the multi-result query path.
- Plumbs
tenantthrough ARG query calls in several tool services (Storage, Kusto, Grafana, Authorization, AppConfig, ACR).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.Storage/src/Services/StorageService.cs | Passes tenant into ARG list/single queries for storage accounts. |
| tools/Azure.Mcp.Tools.Kusto/src/Services/KustoService.cs | Passes tenant into ARG list/single queries for Kusto clusters. |
| tools/Azure.Mcp.Tools.Grafana/src/Services/GrafanaService.cs | Passes tenant into ARG workspace listing. |
| tools/Azure.Mcp.Tools.Authorization/src/Services/AuthorizationService.cs | Passes tenantId into ARG role assignment listing. |
| tools/Azure.Mcp.Tools.AppConfig/src/Services/AppConfigService.cs | Passes tenant into ARG store listing (but not yet into store lookup used by other operations). |
| tools/Azure.Mcp.Tools.Acr/src/Services/AcrService.cs | Passes tenant into ARG registry list/single queries. |
| core/Azure.Mcp.Core/src/Services/Azure/BaseAzureResourceService.cs | Adds tenant support to ARG query execution and refactors single-query path. |
| var subscriptionResource = await _subscriptionService.GetSubscription(subscription, null, retryPolicy, cancellationToken); | ||
| var tenantResource = await GetTenantResourceAsync(subscriptionResource.Data.TenantId, cancellationToken); | ||
| var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy, cancellationToken); | ||
| var tenantResource = await GetTenantResourceAsync(subscriptionResource!.Data.TenantId, cancellationToken); |
There was a problem hiding this comment.
subscriptionResource is declared non-nullable by ISubscriptionService.GetSubscription(...) (it throws if it can't retrieve the subscription). Using the null-forgiving operator here is unnecessary and can mask future nullability issues; prefer removing ! (or, if null is actually possible, handle it explicitly with a clear exception before dereferencing .Data).
| var tenantResource = await GetTenantResourceAsync(subscriptionResource!.Data.TenantId, cancellationToken); | |
| var tenantResource = await GetTenantResourceAsync(subscriptionResource.Data.TenantId, cancellationToken); |
| string? tenant = null, | ||
| CancellationToken cancellationToken = default) where T : class |
There was a problem hiding this comment.
ExecuteSingleResourceQueryAsync introduces tenant before cancellationToken, while ExecuteResourceQueryAsync appends tenant after cancellationToken. This inconsistent ordering makes the API easier to misuse and is source-breaking for any positional callers that previously passed only a cancellationToken. Consider standardizing both methods (ideally keeping cancellationToken last) and updating internal call sites to use named arguments as needed.
| string? tenant = null, | |
| CancellationToken cancellationToken = default) where T : class | |
| CancellationToken cancellationToken = default, | |
| string? tenant = null) where T : class |
| /// <param name="resourceType">The Azure resource type to query for (e.g., "Microsoft.Sql/servers/databases")</param> | ||
| /// <param name="resourceGroup">The resource group name to filter by (null to query all resource groups)</param> | ||
| /// <param name="subscription">The subscription ID or name</param> | ||
| /// <param name="retryPolicy">Optional retry policy configuration</param> | ||
| /// <param name="converter">Function to convert JsonElement to the target type</param> | ||
| /// <param name="additionalFilter">Optional additional KQL filter conditions</param> | ||
| /// <param name="cancellationToken">Cancellation token</param> | ||
| /// <returns>Single resource converted to the specified type, or null if not found</returns> | ||
| protected async Task<T?> ExecuteSingleResourceQueryAsync<T>( | ||
| string resourceType, | ||
| string? resourceGroup, | ||
| string subscription, | ||
| RetryPolicyOptions? retryPolicy, | ||
| Func<JsonElement, T> converter, | ||
| string? tableName = "resources", | ||
| string? additionalFilter = null, | ||
| string? tenant = null, | ||
| CancellationToken cancellationToken = default) where T : class |
There was a problem hiding this comment.
The XML docs for ExecuteSingleResourceQueryAsync no longer match the signature: the method has a tableName parameter and now also has an optional tenant, but neither is documented. Please update the <param> entries so the docs reflect the actual parameters and intended usage.
| var accounts = await ExecuteResourceQueryAsync( | ||
| "Microsoft.AppConfiguration/configurationStores", | ||
| resourceGroup: null, // all resource groups | ||
| subscription, | ||
| retryPolicy, | ||
| ConvertToAppConfigurationAccountModel, | ||
| tenant: tenant, | ||
| cancellationToken: cancellationToken); |
There was a problem hiding this comment.
tenant is now propagated for GetAppConfigAccounts, but other AppConfig operations that accept a tenant (e.g., GetKeyValues via GetConfigurationClient/FindAppConfigStore) still discover the store endpoint via an ARG query that does not pass tenant. This means cross-tenant scenarios can still resolve the subscription/store in the default tenant. Consider threading tenant into FindAppConfigStore(...) and passing it to ExecuteSingleResourceQueryAsync(... tenant: tenant ...), then updating GetConfigurationClient to forward the tenant.
What does this PR do?
Accept
tenantwhen calling Azure Resource Graph queries to allow configuring both tenant and subscription.GitHub issue number?
[Link to the GitHub issue this PR addresses]Pre-merge Checklist
servers/Azure.Mcp.Server/CHANGELOG.mdand/orservers/Fabric.Mcp.Server/CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies)servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationeng/scripts/Process-PackageReadMe.ps1. See Package README/servers/Azure.Mcp.Server/docs/azmcp-commands.mdand/or/docs/fabric-commands.md.\eng\scripts\Update-AzCommandsMetadata.ps1to update tool metadata in azmcp-commands.md (required for CI)ToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.json/servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline