From 72a7f0bc192e3ecdfbb7300c06e83797583d3406 Mon Sep 17 00:00:00 2001 From: Nori Zhang <“norizhang@microsoft.com”> Date: Fri, 9 Sep 2022 09:58:34 +0800 Subject: [PATCH 1/5] Fix the same subscription ids should not be returned in GetSubscriptionListByName issue. --- .../Accounts.Test/AzureRMProfileTests.cs | 25 ++++++++++++++++- .../Mocks/MockSubscriptionClientFactory.cs | 4 +++ .../Accounts/Models/RMProfileClient.cs | 27 ++++++++++++++++--- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/src/Accounts/Accounts.Test/AzureRMProfileTests.cs b/src/Accounts/Accounts.Test/AzureRMProfileTests.cs index d47c9551eed7..66afd5b7de31 100644 --- a/src/Accounts/Accounts.Test/AzureRMProfileTests.cs +++ b/src/Accounts/Accounts.Test/AzureRMProfileTests.cs @@ -500,7 +500,7 @@ public void SingleTenantSubscriptionListSucceed() [Fact] [Trait(Category.AcceptanceType, Category.CheckIn)] - public void GetSubscriptionListByNameCorrect() + public void GetSubscriptionListByNameSameIdCorrect() { var tenants = new List { DefaultTenant.ToString() }; var firstList = new List { DefaultSubscription.ToString() }; @@ -514,6 +514,29 @@ public void GetSubscriptionListByNameCorrect() client.TryGetSubscriptionListByName(DefaultTenant.ToString(), MockSubscriptionClientFactory.GetSubscriptionNameFromId(DefaultSubscription.ToString()), out subValueList); + Assert.Single(subValueList); + } + + + [Fact] + [Trait(Category.AcceptanceType, Category.CheckIn)] + public void GetSubscriptionListByNameCorrect() + { + var subId1 = "a11a11aa-aaaa-aaaa-aaaa-aaaa1111aaaa"; + var subId2 = "aaaa11aa-aaaa-aaaa-aaaa-aaaa1111aaaa"; + + var tenants = new List { DefaultTenant.ToString() }; + var firstList = new List { subId1 }; + var secondList = new List { subId2 }; + var thirdList = firstList; + var fourthList = firstList; + var client = SetupTestEnvironment(tenants, firstList, secondList, thirdList, fourthList); + var tenantResults = client.ListTenants(); + Assert.Single(tenantResults); + IEnumerable subValueList; + client.TryGetSubscriptionListByName(DefaultTenant.ToString(), + "SameNameForGetSubscriptionByName", + out subValueList); Assert.Equal(2, subValueList.Count()); } diff --git a/src/Accounts/Accounts.Test/Mocks/MockSubscriptionClientFactory.cs b/src/Accounts/Accounts.Test/Mocks/MockSubscriptionClientFactory.cs index ff87519ed830..f0315d64a2f4 100644 --- a/src/Accounts/Accounts.Test/Mocks/MockSubscriptionClientFactory.cs +++ b/src/Accounts/Accounts.Test/Mocks/MockSubscriptionClientFactory.cs @@ -54,6 +54,10 @@ public MockSubscriptionClientFactory() public static string GetSubscriptionNameFromId(string id) { + if(id == "a11a11aa-aaaa-aaaa-aaaa-aaaa1111aaaa" || id == "aaaa11aa-aaaa-aaaa-aaaa-aaaa1111aaaa") + { + return "SameNameForGetSubscriptionByName"; + } return "Sub-" + id; } diff --git a/src/Accounts/Accounts/Models/RMProfileClient.cs b/src/Accounts/Accounts/Models/RMProfileClient.cs index 33ff4f3cf529..6a21435286e5 100644 --- a/src/Accounts/Accounts/Models/RMProfileClient.cs +++ b/src/Accounts/Accounts/Models/RMProfileClient.cs @@ -421,11 +421,30 @@ public bool TryGetSubscriptionByName(string tenantId, string subscriptionName, o return subscription != null; } - public bool TryGetSubscriptionListByName(string tenantId, string subscriptionName, out IEnumerable subscriptionList) + public bool TryGetSubscriptionListByName(string tenantId, string subscriptionName, out IEnumerable subscriptions) { - subscriptionList = ListSubscriptions(tenantId); - subscriptionList = subscriptionList.Where(s => s.Name.Equals(subscriptionName, StringComparison.OrdinalIgnoreCase)); - return subscriptionList.Any(); + subscriptions = ListSubscriptions(tenantId).Where(s => s.Name.Equals(subscriptionName, StringComparison.OrdinalIgnoreCase)); + List subscriptionList = new List(); + HashSet subscriptionIds = new HashSet(); + foreach(IAzureSubscription subscription in subscriptions) + { + + if(subscription is PSAzureSubscription && subscription.GetTenant() != null + && subscription.GetHomeTenant().Equals(subscription.GetTenant()) && subscriptionIds.Add(subscription.GetId())) + { + subscriptionList.Add(subscription); + } + + } + foreach (IAzureSubscription subscription in subscriptions) + { + if (subscriptionIds.Add(subscription.GetId())) + { + subscriptionList.Add(subscription); + } + } + subscriptions = subscriptionList; + return subscriptions.Any(); } private IAzureSubscription GetFirstSubscription(string tenantId) From e05199c7b59a6ec73c82316064996c05f808743d Mon Sep 17 00:00:00 2001 From: Nori Zhang <“norizhang@microsoft.com”> Date: Fri, 9 Sep 2022 11:23:18 +0800 Subject: [PATCH 2/5] ChangLog update. --- src/Accounts/Accounts/ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md index 8679528456fe..48c0aaa91714 100644 --- a/src/Accounts/Accounts/ChangeLog.md +++ b/src/Accounts/Accounts/ChangeLog.md @@ -19,7 +19,7 @@ --> ## Upcoming Release - +* Fixed returning duplicate Ids for one subscription while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19427] ## Version 2.10.0 * Supported returning all subscriptions with specified name while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19295] * Fixed null reference exception when cmdlet uses AzureRestOperation [#18104] From 408f7149f87319f1adce002072e21e35e462eeea Mon Sep 17 00:00:00 2001 From: Nori Zhang <“norizhang@microsoft.com”> Date: Fri, 9 Sep 2022 16:32:20 +0800 Subject: [PATCH 3/5] Changelog Update. --- src/Accounts/Accounts/ChangeLog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md index 48c0aaa91714..d3c807d2708b 100644 --- a/src/Accounts/Accounts/ChangeLog.md +++ b/src/Accounts/Accounts/ChangeLog.md @@ -20,6 +20,7 @@ ## Upcoming Release * Fixed returning duplicate Ids for one subscription while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19427] + ## Version 2.10.0 * Supported returning all subscriptions with specified name while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19295] * Fixed null reference exception when cmdlet uses AzureRestOperation [#18104] From b1c6fc8f2c755f86036ac8e719eeb14591bfefbf Mon Sep 17 00:00:00 2001 From: Nori Zhang <“norizhang@microsoft.com”> Date: Fri, 9 Sep 2022 17:01:36 +0800 Subject: [PATCH 4/5] Polished code. --- src/Accounts/Accounts/Models/RMProfileClient.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Accounts/Accounts/Models/RMProfileClient.cs b/src/Accounts/Accounts/Models/RMProfileClient.cs index 6a21435286e5..022c8302691b 100644 --- a/src/Accounts/Accounts/Models/RMProfileClient.cs +++ b/src/Accounts/Accounts/Models/RMProfileClient.cs @@ -425,20 +425,22 @@ public bool TryGetSubscriptionListByName(string tenantId, string subscriptionNam { subscriptions = ListSubscriptions(tenantId).Where(s => s.Name.Equals(subscriptionName, StringComparison.OrdinalIgnoreCase)); List subscriptionList = new List(); - HashSet subscriptionIds = new HashSet(); + HashSet existedSubscriptionIds = new HashSet(); + + // Consider subscription in Home tenant first, exclude duplicate subscriptions by id. foreach(IAzureSubscription subscription in subscriptions) { - - if(subscription is PSAzureSubscription && subscription.GetTenant() != null - && subscription.GetHomeTenant().Equals(subscription.GetTenant()) && subscriptionIds.Add(subscription.GetId())) + if (subscription is PSAzureSubscription && subscription.GetTenant() != null + && subscription.GetHomeTenant().Equals(subscription.GetTenant()) && existedSubscriptionIds.Add(subscription.GetId())) { subscriptionList.Add(subscription); } } + // Consider other subscriptions. foreach (IAzureSubscription subscription in subscriptions) { - if (subscriptionIds.Add(subscription.GetId())) + if (existedSubscriptionIds.Add(subscription.GetId())) { subscriptionList.Add(subscription); } From 4930dea173cd91f0c1486a01e8e03087923fb572 Mon Sep 17 00:00:00 2001 From: Nori Zhang <“norizhang@microsoft.com”> Date: Tue, 13 Sep 2022 16:22:53 +0800 Subject: [PATCH 5/5] Polish ChageLog. --- src/Accounts/Accounts/ChangeLog.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md index d3c807d2708b..37493d2805d1 100644 --- a/src/Accounts/Accounts/ChangeLog.md +++ b/src/Accounts/Accounts/ChangeLog.md @@ -19,7 +19,7 @@ --> ## Upcoming Release -* Fixed returning duplicate Ids for one subscription while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19427] +* Deduplicated subscriptions belonging to multiple tenants while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19427] ## Version 2.10.0 * Supported returning all subscriptions with specified name while using `Get-AzSubscription` with parameter `SubscriptionName`. [#19295]