diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 000000000..f1263afbb --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,41 @@ +{ + "name": "Azure MCP Codespace", + "features": { + "ghcr.io/devcontainers/features/dotnet:2": { + "version": "9.0.300" + }, + "ghcr.io/devcontainers/features/node:1": { + "version": "20" + }, + "ghcr.io/devcontainers/features/azure-cli:1": {}, + "ghcr.io/devcontainers/features/powershell:1": {}, + "ghcr.io/azure/azure-dev/azd:0": {} + }, + "hostRequirements": { + "cpus": 4, + "memory": "8gb", + "storage": "32gb" + }, + "customizations": { + "vscode": { + "extensions": [ + "ms-dotnettools.csharp", + "ms-dotnettools.csdevkit", + "ms-vscode.powershell", + "ms-azuretools.vscode-azurecli", + "GitHub.copilot", + "GitHub.copilot-chat", + "ms-azuretools.vscode-azure-github-copilot", + "dbaeumer.vscode-eslint", + "streetsidesoftware.code-spell-checker", + "ms-vscode.test-adapter-converter" + ], + "settings": { + "dotnet.completion.showCompletionItemsFromUnimportedNamespaces": true, + "dotnet.server.useOmnisharp": false, + "powershell.codeFormatting.preset": "OTBS" + } + } + }, + "postCreateCommand": "dotnet --version && az --version && azd version && pwsh --version && npm --version" +} diff --git a/.vscode/cspell.json b/.vscode/cspell.json index 0361781e3..9d5ca536e 100644 --- a/.vscode/cspell.json +++ b/.vscode/cspell.json @@ -25,7 +25,8 @@ "**/bin/**", "**/.work/**", "**/.dist/**", - "**/eng/vscode/NOTICE.txt" + "**/eng/vscode/NOTICE.txt", + "!**/.devcontainer/**" ], "dictionaryDefinitions": [ { @@ -200,7 +201,9 @@ "azuretools", "codegen", "codeium", + "Codespace", "cslschema", + "csdevkit", "bdylan", "bestpractices", "bicepschema", @@ -217,7 +220,9 @@ "datasource", "datasources", "deallocate", + "devcontainers", "Distributedtask", + "dotnettools", "DEBUGTELEMETRY", "dotenv", "drawcord", diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a1ef2298..ed2514380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ The Azure MCP Server updates automatically by default whenever a new release com ### Bugs Fixed +- Fixed subscription parameter handling across all Azure MCP service methods to consistently use `subscription` instead of `subscriptionId`, enabling proper support for both subscription IDs and subscription names. [[#877](https://github.com/Azure/azure-mcp/issues/877)] - Fixed `ToolExecuted` telemetry activity being created twice. [[#741](https://github.com/Azure/azure-mcp/pull/741)] ### Other Changes diff --git a/areas/appconfig/src/AzureMcp.AppConfig/Services/AppConfigService.cs b/areas/appconfig/src/AzureMcp.AppConfig/Services/AppConfigService.cs index ada457491..9da14a374 100644 --- a/areas/appconfig/src/AzureMcp.AppConfig/Services/AppConfigService.cs +++ b/areas/appconfig/src/AzureMcp.AppConfig/Services/AppConfigService.cs @@ -19,14 +19,14 @@ public class AppConfigService(ISubscriptionService subscriptionService, ITenantS { private readonly ISubscriptionService _subscriptionService = subscriptionService ?? throw new ArgumentNullException(nameof(subscriptionService)); - public async Task> GetAppConfigAccounts(string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null) + public async Task> GetAppConfigAccounts(string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId); + ValidateRequiredParameters(subscription); - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy); var accounts = new List(); - await foreach (var account in subscription.GetAppConfigurationStoresAsync()) + await foreach (var account in subscriptionResource.GetAppConfigurationStoresAsync()) { ResourceIdentifier resourceId = account.Id; if (resourceId.ToString().Length == 0) @@ -81,15 +81,15 @@ public async Task> GetAppConfigAccounts(string sub public async Task> ListKeyValues( string accountName, - string subscriptionId, + string subscription, string? key = null, string? label = null, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, subscriptionId); + ValidateRequiredParameters(accountName, subscription); - var client = await GetConfigurationClient(accountName, subscriptionId, tenant, retryPolicy); + var client = await GetConfigurationClient(accountName, subscription, tenant, retryPolicy); var settings = new List(); var selector = new SettingSelector @@ -115,10 +115,10 @@ public async Task> ListKeyValues( return settings; } - public async Task GetKeyValue(string accountName, string key, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null, string? contentType = null) + public async Task GetKeyValue(string accountName, string key, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null, string? contentType = null) { - ValidateRequiredParameters(accountName, key, subscriptionId); - var client = await GetConfigurationClient(accountName, subscriptionId, tenant, retryPolicy); + ValidateRequiredParameters(accountName, key, subscription); + var client = await GetConfigurationClient(accountName, subscription, tenant, retryPolicy); var response = await client.GetConfigurationSettingAsync(key, label, cancellationToken: default); var setting = response.Value; @@ -134,20 +134,20 @@ public async Task GetKeyValue(string accountName, string key, s }; } - public async Task LockKeyValue(string accountName, string key, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null) + public async Task LockKeyValue(string accountName, string key, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null) { - await SetKeyValueReadOnlyState(accountName, key, subscriptionId, tenant, retryPolicy, label, true); + await SetKeyValueReadOnlyState(accountName, key, subscription, tenant, retryPolicy, label, true); } - public async Task UnlockKeyValue(string accountName, string key, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null) + public async Task UnlockKeyValue(string accountName, string key, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null) { - await SetKeyValueReadOnlyState(accountName, key, subscriptionId, tenant, retryPolicy, label, false); + await SetKeyValueReadOnlyState(accountName, key, subscription, tenant, retryPolicy, label, false); } - public async Task SetKeyValue(string accountName, string key, string value, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null, string? contentType = null, string[]? tags = null) + public async Task SetKeyValue(string accountName, string key, string value, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null, string? contentType = null, string[]? tags = null) { - ValidateRequiredParameters(accountName, key, value, subscriptionId); - var client = await GetConfigurationClient(accountName, subscriptionId, tenant, retryPolicy); + ValidateRequiredParameters(accountName, key, value, subscription); + var client = await GetConfigurationClient(accountName, subscription, tenant, retryPolicy); // Create a ConfigurationSetting object to include contentType if provided var setting = new ConfigurationSetting(key, value, label) @@ -179,24 +179,24 @@ public async Task SetKeyValue(string accountName, string key, string value, stri await client.SetConfigurationSettingAsync(setting, cancellationToken: default); } - public async Task DeleteKeyValue(string accountName, string key, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null) + public async Task DeleteKeyValue(string accountName, string key, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null) { - ValidateRequiredParameters(accountName, key, subscriptionId); - var client = await GetConfigurationClient(accountName, subscriptionId, tenant, retryPolicy); + ValidateRequiredParameters(accountName, key, subscription); + var client = await GetConfigurationClient(accountName, subscription, tenant, retryPolicy); await client.DeleteConfigurationSettingAsync(key, label, cancellationToken: default); } - private async Task SetKeyValueReadOnlyState(string accountName, string key, string subscriptionId, string? tenant, RetryPolicyOptions? retryPolicy, string? label, bool isReadOnly) + private async Task SetKeyValueReadOnlyState(string accountName, string key, string subscription, string? tenant, RetryPolicyOptions? retryPolicy, string? label, bool isReadOnly) { - ValidateRequiredParameters(accountName, key, subscriptionId); - var client = await GetConfigurationClient(accountName, subscriptionId, tenant, retryPolicy); + ValidateRequiredParameters(accountName, key, subscription); + var client = await GetConfigurationClient(accountName, subscription, tenant, retryPolicy); await client.SetReadOnlyAsync(key, label, isReadOnly, cancellationToken: default); } - private async Task GetConfigurationClient(string accountName, string subscriptionId, string? tenant, RetryPolicyOptions? retryPolicy) + private async Task GetConfigurationClient(string accountName, string subscription, string? tenant, RetryPolicyOptions? retryPolicy) { - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy); - var configStore = await FindAppConfigStore(subscription, accountName, subscriptionId); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy); + var configStore = await FindAppConfigStore(subscriptionResource, accountName, subscription); var endpoint = configStore.Data.Endpoint; var credential = await GetCredential(tenant); AddDefaultPolicies(new ConfigurationClientOptions()); @@ -204,7 +204,7 @@ private async Task GetConfigurationClient(string accountNam return new ConfigurationClient(new Uri(endpoint), credential); } - private static async Task FindAppConfigStore(SubscriptionResource subscription, string accountName, string subscriptionId) + private static async Task FindAppConfigStore(SubscriptionResource subscription, string accountName, string subscriptionIdentifier) { AppConfigurationStoreResource? configStore = null; await foreach (var store in subscription.GetAppConfigurationStoresAsync()) @@ -217,7 +217,7 @@ private static async Task FindAppConfigStore(Subs } if (configStore == null) - throw new Exception($"App Configuration store '{accountName}' not found in subscription '{subscriptionId}'"); + throw new Exception($"App Configuration store '{accountName}' not found in subscription '{subscriptionIdentifier}'"); return configStore; } diff --git a/areas/appconfig/src/AzureMcp.AppConfig/Services/IAppConfigService.cs b/areas/appconfig/src/AzureMcp.AppConfig/Services/IAppConfigService.cs index 1b7172e17..f46dcdab2 100644 --- a/areas/appconfig/src/AzureMcp.AppConfig/Services/IAppConfigService.cs +++ b/areas/appconfig/src/AzureMcp.AppConfig/Services/IAppConfigService.cs @@ -9,19 +9,19 @@ namespace AzureMcp.AppConfig.Services; public interface IAppConfigService { Task> GetAppConfigAccounts( - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task> ListKeyValues( string accountName, - string subscriptionId, + string subscription, string? key = null, string? label = null, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task GetKeyValue( string accountName, string key, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null, @@ -29,14 +29,14 @@ Task GetKeyValue( Task LockKeyValue( string accountName, string key, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null); Task UnlockKeyValue( string accountName, string key, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null); @@ -44,7 +44,7 @@ Task SetKeyValue( string accountName, string key, string value, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null, @@ -53,7 +53,7 @@ Task SetKeyValue( Task DeleteKeyValue( string accountName, string key, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null, string? label = null); diff --git a/areas/cosmos/src/AzureMcp.Cosmos/Services/CosmosService.cs b/areas/cosmos/src/AzureMcp.Cosmos/Services/CosmosService.cs index 78b183c8c..a36d16ff6 100644 --- a/areas/cosmos/src/AzureMcp.Cosmos/Services/CosmosService.cs +++ b/areas/cosmos/src/AzureMcp.Cosmos/Services/CosmosService.cs @@ -25,28 +25,28 @@ public class CosmosService(ISubscriptionService subscriptionService, ITenantServ private bool _disposed; private async Task GetCosmosAccountAsync( - string subscriptionId, + string subscription, string accountName, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId, accountName); + ValidateRequiredParameters(subscription, accountName); - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy); - await foreach (var account in subscription.GetCosmosDBAccountsAsync()) + await foreach (var account in subscriptionResource.GetCosmosDBAccountsAsync()) { if (account.Data.Name == accountName) { return account; } } - throw new Exception($"Cosmos DB account '{accountName}' not found in subscription '{subscriptionId}'"); + throw new Exception($"Cosmos DB account '{accountName}' not found in subscription '{subscription}'"); } private async Task CreateCosmosClientWithAuth( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod, string? tenant = null, RetryPolicyOptions? retryPolicy = null) @@ -65,7 +65,7 @@ private async Task CreateCosmosClientWithAuth( switch (authMethod) { case AuthMethod.Key: - var cosmosAccount = await GetCosmosAccountAsync(subscriptionId, accountName, tenant); + var cosmosAccount = await GetCosmosAccountAsync(subscription, accountName, tenant); var keys = await cosmosAccount.GetKeysAsync(); cosmosClient = new CosmosClient( string.Format(CosmosBaseUri, accountName), @@ -107,12 +107,12 @@ private async Task ValidateCosmosClientAsync(CosmosClient client) private async Task GetCosmosClientAsync( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, subscriptionId); + ValidateRequiredParameters(accountName, subscription); var key = CosmosClientsCacheKeyPrefix + accountName; var cosmosClient = await _cacheService.GetAsync(CacheGroup, key, s_cacheDurationResources); @@ -124,7 +124,7 @@ private async Task GetCosmosClientAsync( // First attempt with requested auth method cosmosClient = await CreateCosmosClientWithAuth( accountName, - subscriptionId, + subscription, authMethod, tenant, retryPolicy); @@ -139,7 +139,7 @@ private async Task GetCosmosClientAsync( // If credential auth fails with 401/403, try key auth cosmosClient = await CreateCosmosClientWithAuth( accountName, - subscriptionId, + subscription, AuthMethod.Key, tenant, retryPolicy); @@ -151,15 +151,15 @@ private async Task GetCosmosClientAsync( throw new Exception($"Failed to create Cosmos client for account '{accountName}' with any authentication method"); } - public async Task> GetCosmosAccounts(string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null) + public async Task> GetCosmosAccounts(string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId); + ValidateRequiredParameters(subscription); - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy); var accounts = new List(); try { - await foreach (var account in subscription.GetCosmosDBAccountsAsync()) + await foreach (var account in subscriptionResource.GetCosmosDBAccountsAsync()) { if (account?.Data?.Name != null) { @@ -177,12 +177,12 @@ public async Task> GetCosmosAccounts(string subscriptionId, string? public async Task> ListDatabases( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, subscriptionId); + ValidateRequiredParameters(accountName, subscription); var cacheKey = CosmosDatabasesCacheKeyPrefix + accountName; @@ -192,7 +192,7 @@ public async Task> ListDatabases( return cachedDatabases; } - var client = await GetCosmosClientAsync(accountName, subscriptionId, authMethod, tenant, retryPolicy); + var client = await GetCosmosClientAsync(accountName, subscription, authMethod, tenant, retryPolicy); var databases = new List(); try @@ -216,12 +216,12 @@ public async Task> ListDatabases( public async Task> ListContainers( string accountName, string databaseName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, databaseName, subscriptionId); + ValidateRequiredParameters(accountName, databaseName, subscription); var cacheKey = CosmosContainersCacheKeyPrefix + accountName + "_" + databaseName; @@ -231,7 +231,7 @@ public async Task> ListContainers( return cachedContainers; } - var client = await GetCosmosClientAsync(accountName, subscriptionId, authMethod, tenant, retryPolicy); + var client = await GetCosmosClientAsync(accountName, subscription, authMethod, tenant, retryPolicy); var containers = new List(); try @@ -258,14 +258,14 @@ public async Task> QueryItems( string databaseName, string containerName, string? query, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, databaseName, containerName, subscriptionId); + ValidateRequiredParameters(accountName, databaseName, containerName, subscription); - var client = await GetCosmosClientAsync(accountName, subscriptionId, authMethod, tenant, retryPolicy); + var client = await GetCosmosClientAsync(accountName, subscription, authMethod, tenant, retryPolicy); try { diff --git a/areas/cosmos/src/AzureMcp.Cosmos/Services/ICosmosService.cs b/areas/cosmos/src/AzureMcp.Cosmos/Services/ICosmosService.cs index c5d1a8eef..9c780b926 100644 --- a/areas/cosmos/src/AzureMcp.Cosmos/Services/ICosmosService.cs +++ b/areas/cosmos/src/AzureMcp.Cosmos/Services/ICosmosService.cs @@ -10,13 +10,13 @@ namespace AzureMcp.Cosmos.Services; public interface ICosmosService : IDisposable { Task> GetCosmosAccounts( - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task> ListDatabases( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -24,7 +24,7 @@ Task> ListDatabases( Task> ListContainers( string accountName, string databaseName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -34,7 +34,7 @@ Task> QueryItems( string databaseName, string containerName, string? query, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? tenant = null, RetryPolicyOptions? retryPolicy = null); diff --git a/areas/grafana/src/AzureMcp.Grafana/Services/GrafanaService.cs b/areas/grafana/src/AzureMcp.Grafana/Services/GrafanaService.cs index b872a5185..95bb91522 100644 --- a/areas/grafana/src/AzureMcp.Grafana/Services/GrafanaService.cs +++ b/areas/grafana/src/AzureMcp.Grafana/Services/GrafanaService.cs @@ -15,15 +15,15 @@ public class GrafanaService(ISubscriptionService _subscriptionService, ITenantSe : BaseAzureService(tenantService), IGrafanaService { public async Task> ListWorkspacesAsync( - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId); + ValidateRequiredParameters(subscription); try { - var subscriptionResource = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscriptionId}' not found"); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscription}' not found"); var workspaces = new List(); await foreach (var workspaceResource in subscriptionResource.GetManagedGrafanasAsync()) diff --git a/areas/kusto/src/AzureMcp.Kusto/Services/IKustoService.cs b/areas/kusto/src/AzureMcp.Kusto/Services/IKustoService.cs index 0e4b7e77a..c1a2f8db8 100644 --- a/areas/kusto/src/AzureMcp.Kusto/Services/IKustoService.cs +++ b/areas/kusto/src/AzureMcp.Kusto/Services/IKustoService.cs @@ -9,12 +9,12 @@ namespace AzureMcp.Kusto.Services; public interface IKustoService { Task> ListClusters( - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task GetCluster( - string subscriptionId, + string subscription, string clusterName, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -26,7 +26,7 @@ Task> ListDatabases( RetryPolicyOptions? retryPolicy = null); Task> ListDatabases( - string subscriptionId, + string subscription, string clusterName, string? tenant = null, AuthMethod? authMethod = AuthMethod.Credential, diff --git a/areas/redis/src/AzureMcp.Redis/Services/IRedisService.cs b/areas/redis/src/AzureMcp.Redis/Services/IRedisService.cs index 11288898f..65be6b44f 100644 --- a/areas/redis/src/AzureMcp.Redis/Services/IRedisService.cs +++ b/areas/redis/src/AzureMcp.Redis/Services/IRedisService.cs @@ -54,7 +54,7 @@ Task> ListClustersAsync( Task> ListDatabasesAsync( string clusterName, string resourceGroupName, - string subscriptionId, + string subscription, string? tenant = null, AuthMethod? authMethod = null, RetryPolicyOptions? retryPolicy = null); @@ -73,7 +73,7 @@ Task> ListDatabasesAsync( Task> ListAccessPolicyAssignmentsAsync( string cacheName, string resourceGroupName, - string subscriptionId, + string subscription, string? tenant = null, AuthMethod? authMethod = null, RetryPolicyOptions? retryPolicy = null); diff --git a/areas/redis/src/AzureMcp.Redis/Services/RedisService.cs b/areas/redis/src/AzureMcp.Redis/Services/RedisService.cs index fba4dff7b..2437a767a 100644 --- a/areas/redis/src/AzureMcp.Redis/Services/RedisService.cs +++ b/areas/redis/src/AzureMcp.Redis/Services/RedisService.cs @@ -19,16 +19,16 @@ public class RedisService(ISubscriptionService _subscriptionService, IResourceGr : BaseAzureService(tenantService), IRedisService { public async Task> ListCachesAsync( - string subscriptionId, + string subscription, string? tenant = null, AuthMethod? authMethod = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId); + ValidateRequiredParameters(subscription); try { - var subscriptionResource = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscriptionId}' not found"); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscription}' not found"); var caches = new List(); await foreach (var cacheResource in subscriptionResource.GetAllRedisAsync()) @@ -117,16 +117,16 @@ public async Task> ListCachesAsync( public async Task> ListAccessPolicyAssignmentsAsync( string cacheName, string resourceGroupName, - string subscriptionId, + string subscription, string? tenant = null, AuthMethod? authMethod = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(cacheName, resourceGroupName, subscriptionId); + ValidateRequiredParameters(cacheName, resourceGroupName, subscription); try { - var resourceGroup = await _resourceGroupService.GetResourceGroupResource(subscriptionId, resourceGroupName, tenant, retryPolicy) ?? throw new Exception($"Resource group named '{resourceGroupName}' not found"); + var resourceGroup = await _resourceGroupService.GetResourceGroupResource(subscription, resourceGroupName, tenant, retryPolicy) ?? throw new Exception($"Resource group named '{resourceGroupName}' not found"); var cacheResponse = await resourceGroup.GetRedisAsync(cacheName); var accessPolicyAssignmentCollection = cacheResponse.Value.GetRedisCacheAccessPolicyAssignments(); var accessPolicyAssignments = new List(); @@ -157,16 +157,16 @@ public async Task> ListAccessPolicyAssignmen } public async Task> ListClustersAsync( - string subscriptionId, + string subscription, string? tenant = null, AuthMethod? authMethod = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId); + ValidateRequiredParameters(subscription); try { - var subscriptionResource = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscriptionId}' not found"); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscription}' not found"); var clusters = new List(); await foreach (var clusterResource in subscriptionResource.GetRedisEnterpriseClustersAsync()) @@ -224,16 +224,16 @@ public async Task> ListClustersAsync( public async Task> ListDatabasesAsync( string clusterName, string resourceGroupName, - string subscriptionId, + string subscription, string? tenant = null, AuthMethod? authMethod = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(clusterName, resourceGroupName, subscriptionId); + ValidateRequiredParameters(clusterName, resourceGroupName, subscription); try { - var resourceGroup = await _resourceGroupService.GetResourceGroupResource(subscriptionId, resourceGroupName, tenant, retryPolicy) ?? throw new Exception($"Resource group named '{resourceGroupName}' not found"); + var resourceGroup = await _resourceGroupService.GetResourceGroupResource(subscription, resourceGroupName, tenant, retryPolicy) ?? throw new Exception($"Resource group named '{resourceGroupName}' not found"); var clusterResponse = await resourceGroup.GetRedisEnterpriseClusterAsync(clusterName); var databaseCollection = clusterResponse.Value.GetRedisEnterpriseDatabases(); var databases = new List(); diff --git a/areas/redis/tests/AzureMcp.Redis.UnitTests/CacheForRedis/AccessPolicyListCommandTests.cs b/areas/redis/tests/AzureMcp.Redis.UnitTests/CacheForRedis/AccessPolicyListCommandTests.cs index 6931ac698..f11385c76 100644 --- a/areas/redis/tests/AzureMcp.Redis.UnitTests/CacheForRedis/AccessPolicyListCommandTests.cs +++ b/areas/redis/tests/AzureMcp.Redis.UnitTests/CacheForRedis/AccessPolicyListCommandTests.cs @@ -103,12 +103,12 @@ public async Task ExecuteAsync_HandlesException() { var expectedError = "Test error. To mitigate this issue, please refer to the troubleshooting guidelines here at https://aka.ms/azmcp/troubleshooting."; _redisService.ListAccessPolicyAssignmentsAsync( - cacheName: "cache1", - resourceGroupName: "rg1", - subscriptionId: "sub123", - tenant: Arg.Any(), - authMethod: Arg.Any(), - retryPolicy: Arg.Any()) + "cache1", + "rg1", + "sub123", + Arg.Any(), + Arg.Any(), + Arg.Any()) .ThrowsAsync(new Exception("Test error")); var command = new AccessPolicyListCommand(_logger); diff --git a/areas/storage/src/AzureMcp.Storage/Services/IStorageService.cs b/areas/storage/src/AzureMcp.Storage/Services/IStorageService.cs index 69b9eb6e6..58a40af92 100644 --- a/areas/storage/src/AzureMcp.Storage/Services/IStorageService.cs +++ b/areas/storage/src/AzureMcp.Storage/Services/IStorageService.cs @@ -9,18 +9,18 @@ namespace AzureMcp.Storage.Services; public interface IStorageService { - Task> GetStorageAccounts(string subscriptionId, + Task> GetStorageAccounts(string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task> ListContainers(string accountName, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task> ListTables( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? connectionString = null, string? tenant = null, @@ -28,14 +28,14 @@ Task> ListTables( Task> ListBlobs(string accountName, string containerName, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); Task GetContainerDetails( string accountName, string containerName, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -43,7 +43,7 @@ Task> ListDataLakePaths( string accountName, string fileSystemName, bool recursive, - string subscriptionId, + string subscription, string? filterPath = null, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -51,7 +51,7 @@ Task> ListDataLakePaths( Task CreateDirectory( string accountName, string directoryPath, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -60,7 +60,7 @@ Task CreateDirectory( string containerName, string tier, string[] blobNames, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); @@ -69,7 +69,7 @@ Task> ListFilesAndDirectories( string shareName, string directoryPath, string? prefix, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); } diff --git a/areas/storage/src/AzureMcp.Storage/Services/StorageService.cs b/areas/storage/src/AzureMcp.Storage/Services/StorageService.cs index 956d974a9..aa2c93da3 100644 --- a/areas/storage/src/AzureMcp.Storage/Services/StorageService.cs +++ b/areas/storage/src/AzureMcp.Storage/Services/StorageService.cs @@ -29,14 +29,16 @@ public class StorageService(ISubscriptionService subscriptionService, ITenantSer private const string StorageAccountsCacheKey = "accounts"; private static readonly TimeSpan s_cacheDuration = TimeSpan.FromHours(1); - public async Task> GetStorageAccounts(string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null) + public async Task> GetStorageAccounts(string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(subscriptionId); + ValidateRequiredParameters(subscription); - // Create cache key + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy); + + // Create cache key using the resolved subscription ID for consistency var cacheKey = string.IsNullOrEmpty(tenant) - ? $"{StorageAccountsCacheKey}_{subscriptionId}" - : $"{StorageAccountsCacheKey}_{subscriptionId}_{tenant}"; + ? $"{StorageAccountsCacheKey}_{subscriptionResource.Data.SubscriptionId}" + : $"{StorageAccountsCacheKey}_{subscriptionResource.Data.SubscriptionId}_{tenant}"; // Try to get from cache first var cachedAccounts = await _cacheService.GetAsync>(CacheGroup, cacheKey, s_cacheDuration); @@ -45,11 +47,10 @@ public async Task> GetStorageAccounts(string subscriptionId, string return cachedAccounts; } - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy); var accounts = new List(); try { - await foreach (var account in subscription.GetStorageAccountsAsync()) + await foreach (var account in subscriptionResource.GetStorageAccountsAsync()) { if (account?.Data?.Name != null) { @@ -68,9 +69,9 @@ public async Task> GetStorageAccounts(string subscriptionId, string return accounts; } - public async Task> ListContainers(string accountName, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null) + public async Task> ListContainers(string accountName, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, subscriptionId); + ValidateRequiredParameters(accountName, subscription); var blobServiceClient = await CreateBlobServiceClient(accountName, tenant, retryPolicy); var containers = new List(); @@ -92,13 +93,13 @@ public async Task> ListContainers(string accountName, string subscr public async Task> ListTables( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod = AuthMethod.Credential, string? connectionString = null, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, subscriptionId); + ValidateRequiredParameters(accountName, subscription); var tables = new List(); @@ -107,7 +108,7 @@ public async Task> ListTables( // First attempt with requested auth method var tableServiceClient = await CreateTableServiceClientWithAuth( accountName, - subscriptionId, + subscription, authMethod, connectionString, tenant, @@ -128,7 +129,7 @@ ex is RequestFailedException rfEx && { // If credential auth fails with 403/401, try key auth var keyClient = await CreateTableServiceClientWithAuth( - accountName, subscriptionId, AuthMethod.Key, connectionString, tenant, retryPolicy); + accountName, subscription, AuthMethod.Key, connectionString, tenant, retryPolicy); tables.Clear(); // Reset the list for reuse await foreach (var table in keyClient.QueryAsync()) @@ -141,7 +142,7 @@ ex is RequestFailedException rfEx && { // If key auth fails with 403, try connection string var connStringClient = await CreateTableServiceClientWithAuth( - accountName, subscriptionId, AuthMethod.ConnectionString, connectionString, tenant, retryPolicy); + accountName, subscription, AuthMethod.ConnectionString, connectionString, tenant, retryPolicy); tables.Clear(); // Reset the list for reuse await foreach (var table in connStringClient.QueryAsync()) @@ -163,7 +164,7 @@ ex is RequestFailedException rfEx && { // If key auth fails with 403, try connection string var connStringClient = await CreateTableServiceClientWithAuth( - accountName, subscriptionId, AuthMethod.ConnectionString, connectionString, tenant, retryPolicy); + accountName, subscription, AuthMethod.ConnectionString, connectionString, tenant, retryPolicy); tables.Clear(); // Reset the list for reuse await foreach (var table in connStringClient.QueryAsync()) @@ -183,9 +184,9 @@ ex is RequestFailedException rfEx && } } - public async Task> ListBlobs(string accountName, string containerName, string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null) + public async Task> ListBlobs(string accountName, string containerName, string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, containerName, subscriptionId); + ValidateRequiredParameters(accountName, containerName, subscription); var blobServiceClient = await CreateBlobServiceClient(accountName, tenant, retryPolicy); var containerClient = blobServiceClient.GetBlobContainerClient(containerName); @@ -209,7 +210,7 @@ public async Task> ListBlobs(string accountName, string containerNa public async Task GetContainerDetails( string accountName, string containerName, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { @@ -229,11 +230,11 @@ public async Task GetContainerDetails( } } - private async Task GetStorageAccountKey(string accountName, string subscriptionId, string? tenant = null) + private async Task GetStorageAccountKey(string accountName, string subscription, string? tenant = null) { - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant); - var storageAccount = await GetStorageAccount(subscription, accountName) ?? - throw new Exception($"Storage account '{accountName}' not found in subscription '{subscriptionId}'"); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant); + var storageAccount = await GetStorageAccount(subscriptionResource, accountName) ?? + throw new Exception($"Storage account '{accountName}' not found in subscription '{subscription}'"); var keys = new List(); await foreach (var key in storageAccount.GetKeysAsync()) @@ -245,11 +246,11 @@ private async Task GetStorageAccountKey(string accountName, string subsc return firstKey.Value; } - private async Task GetStorageAccountConnectionString(string accountName, string subscriptionId, string? tenant = null) + private async Task GetStorageAccountConnectionString(string accountName, string subscription, string? tenant = null) { - var subscription = await _subscriptionService.GetSubscription(subscriptionId, tenant); - var storageAccount = await GetStorageAccount(subscription, accountName) ?? - throw new Exception($"Storage account '{accountName}' not found in subscription '{subscriptionId}'"); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant); + var storageAccount = await GetStorageAccount(subscriptionResource, accountName) ?? + throw new Exception($"Storage account '{accountName}' not found in subscription '{subscription}'"); var keys = new List(); await foreach (var key in storageAccount.GetKeysAsync()) @@ -276,7 +277,7 @@ private async Task GetStorageAccountConnectionString(string accountName, protected async Task CreateTableServiceClientWithAuth( string accountName, - string subscriptionId, + string subscription, AuthMethod authMethod, string? connectionString = null, string? tenant = null, @@ -287,12 +288,12 @@ protected async Task CreateTableServiceClientWithAuth( switch (authMethod) { case AuthMethod.Key: - var key = await GetStorageAccountKey(accountName, subscriptionId, tenant); + var key = await GetStorageAccountKey(accountName, subscription, tenant); var uri = $"https://{accountName}.table.core.windows.net"; return new TableServiceClient(new Uri(uri), new TableSharedKeyCredential(accountName, key), options); case AuthMethod.ConnectionString: - var connString = await GetStorageAccountConnectionString(accountName, subscriptionId, tenant); + var connString = await GetStorageAccountConnectionString(accountName, subscription, tenant); return new TableServiceClient(connString, options); case AuthMethod.Credential: @@ -328,12 +329,12 @@ public async Task> ListDataLakePaths( string accountName, string fileSystemName, bool recursive, - string subscriptionId, + string subscription, string? filterPath = null, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, fileSystemName, subscriptionId); + ValidateRequiredParameters(accountName, fileSystemName, subscription); var dataLakeServiceClient = await CreateDataLakeServiceClient(accountName, tenant, retryPolicy); var fileSystemClient = dataLakeServiceClient.GetFileSystemClient(fileSystemName); @@ -364,11 +365,11 @@ public async Task> ListDataLakePaths( public async Task CreateDirectory( string accountName, string directoryPath, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, directoryPath, subscriptionId); + ValidateRequiredParameters(accountName, directoryPath, subscription); var dataLakeServiceClient = await CreateDataLakeServiceClient(accountName, tenant, retryPolicy); @@ -421,11 +422,11 @@ public async Task CreateDirectory( string containerName, string tier, string[] blobNames, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, containerName, tier, subscriptionId); + ValidateRequiredParameters(accountName, containerName, tier, subscription); if (blobNames == null || blobNames.Length == 0) { @@ -492,11 +493,11 @@ public async Task> ListFilesAndDirectories( string shareName, string directoryPath, string? prefix, - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null) { - ValidateRequiredParameters(accountName, shareName, directoryPath, subscriptionId); + ValidateRequiredParameters(accountName, shareName, directoryPath, subscription); var shareServiceClient = await CreateShareServiceClient(accountName, tenant, retryPolicy); diff --git a/areas/workbooks/src/AzureMcp.Workbooks/Services/IWorkbooksService.cs b/areas/workbooks/src/AzureMcp.Workbooks/Services/IWorkbooksService.cs index 94db14907..0dea77b76 100644 --- a/areas/workbooks/src/AzureMcp.Workbooks/Services/IWorkbooksService.cs +++ b/areas/workbooks/src/AzureMcp.Workbooks/Services/IWorkbooksService.cs @@ -8,8 +8,8 @@ namespace AzureMcp.Workbooks.Services; public interface IWorkbooksService { - Task> ListWorkbooks(string subscriptionId, string resourceGroupName, WorkbookFilters? filters = null, RetryPolicyOptions? retryPolicy = null, string? tenant = null); - Task CreateWorkbook(string subscriptionId, string resourceGroupName, string displayName, string serializedData, string sourceId, RetryPolicyOptions? retryPolicy = null, string? tenant = null); + Task> ListWorkbooks(string subscription, string resourceGroupName, WorkbookFilters? filters = null, RetryPolicyOptions? retryPolicy = null, string? tenant = null); + Task CreateWorkbook(string subscription, string resourceGroupName, string displayName, string serializedData, string sourceId, RetryPolicyOptions? retryPolicy = null, string? tenant = null); Task GetWorkbook(string workbookId, RetryPolicyOptions? retryPolicy = null, string? tenant = null); Task UpdateWorkbook(string workbookId, string? displayName = null, string? serializedContent = null, RetryPolicyOptions? retryPolicy = null, string? tenant = null); Task DeleteWorkbook(string workbookId, RetryPolicyOptions? retryPolicy = null, string? tenant = null); diff --git a/areas/workbooks/src/AzureMcp.Workbooks/Services/WorkbooksService.cs b/areas/workbooks/src/AzureMcp.Workbooks/Services/WorkbooksService.cs index 92c573a1c..ad227dc81 100644 --- a/areas/workbooks/src/AzureMcp.Workbooks/Services/WorkbooksService.cs +++ b/areas/workbooks/src/AzureMcp.Workbooks/Services/WorkbooksService.cs @@ -21,12 +21,16 @@ public class WorkbooksService(ISubscriptionService _subscriptionService, ITenant private readonly ILogger _logger = logger; private readonly ITenantService _tenantService = tenantService; - public async Task> ListWorkbooks(string subscriptionId, string resourceGroupName, WorkbookFilters? filters = null, RetryPolicyOptions? retryPolicy = null, string? tenant = null) + public async Task> ListWorkbooks(string subscription, string resourceGroupName, WorkbookFilters? filters = null, RetryPolicyOptions? retryPolicy = null, string? tenant = null) { - ValidateRequiredParameters(subscriptionId, resourceGroupName); + ValidateRequiredParameters(subscription, resourceGroupName); try { + // Resolve subscription to get the actual subscription ID for the query + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy); + var subscriptionId = subscriptionResource.Data.SubscriptionId; + var armClient = await CreateArmClientAsync(tenant, retryPolicy); var tenants = await _tenantService.GetTenants(); @@ -74,7 +78,7 @@ public async Task> ListWorkbooks(string subscriptionId, strin } catch (Exception ex) { - _logger.LogError(ex, "Failed to list workbooks in resource group '{ResourceGroup}' for subscription '{Subscription}'", resourceGroupName, subscriptionId); + _logger.LogError(ex, "Failed to list workbooks in resource group '{ResourceGroup}' for subscription '{Subscription}'", resourceGroupName, subscription); throw new Exception($"Failed to list workbooks: {ex.Message}", ex); } } @@ -200,19 +204,19 @@ public async Task> ListWorkbooks(string subscriptionId, strin } } - public async Task CreateWorkbook(string subscriptionId, string resourceGroupName, string displayName, string serializedData, string sourceId, RetryPolicyOptions? retryPolicy = null, string? tenant = null) + public async Task CreateWorkbook(string subscription, string resourceGroupName, string displayName, string serializedData, string sourceId, RetryPolicyOptions? retryPolicy = null, string? tenant = null) { - ValidateRequiredParameters(subscriptionId, resourceGroupName, displayName, serializedData, sourceId); + ValidateRequiredParameters(subscription, resourceGroupName, displayName, serializedData, sourceId); try { // Get the subscription resource - var subscriptionResource = await _subscriptionService.GetSubscription(subscriptionId, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscriptionId}' not found"); + var subscriptionResource = await _subscriptionService.GetSubscription(subscription, tenant, retryPolicy) ?? throw new Exception($"Subscription '{subscription}' not found"); // Get the resource group var resourceGroupResource = await subscriptionResource.GetResourceGroups().GetAsync(resourceGroupName); if (resourceGroupResource?.Value == null) { - throw new Exception($"Resource group '{resourceGroupName}' not found in subscription '{subscriptionId}'"); + throw new Exception($"Resource group '{resourceGroupName}' not found in subscription '{subscription}'"); } // Create the workbook data @@ -314,13 +318,13 @@ public async Task DeleteWorkbook(string workbookId, RetryPolicyOptions? re /// /// Builds a KQL query for retrieving workbooks with optional filters. /// - private static string BuildWorkbooksQuery(string subscriptionId, string resourceGroupName, WorkbookFilters? filters) + private static string BuildWorkbooksQuery(string subscriptionIdentifier, string resourceGroupName, WorkbookFilters? filters) { var queryText = $@" resources | where type == 'microsoft.insights/workbooks' | where resourceGroup =~ '{resourceGroupName}' - | where subscriptionId =~ '{subscriptionId}'"; + | where subscriptionId =~ '{subscriptionIdentifier}'"; // Add optional filters if provided if (filters?.HasFilters == true) diff --git a/docs/new-command.md b/docs/new-command.md index 0e583ad06..6a2e604ed 100644 --- a/docs/new-command.md +++ b/docs/new-command.md @@ -319,12 +319,12 @@ All interface methods should follow consistent formatting with proper line break ```csharp // Correct formatting - parameters aligned with line breaks Task> GetStorageAccounts( - string subscriptionId, + string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); // Incorrect formatting - all parameters on single line -Task> GetStorageAccounts(string subscriptionId, string? tenant = null, RetryPolicyOptions? retryPolicy = null); +Task> GetStorageAccounts(string subscription, string? tenant = null, RetryPolicyOptions? retryPolicy = null); ``` **Formatting Rules:**