Skip to content

Handle invalid locations for Get-AzureRmResourceProvider. #2380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 3, 2016
Merged

Handle invalid locations for Get-AzureRmResourceProvider. #2380

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
21dc96b
Handle invalid locations for Get-AzureRmResourceProvider.
Tiano2017 Jun 2, 2016
0c03564
Merge branch 'dev' of https://github.com/Azure/azure-powershell into …
Tiano2017 Jun 2, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions ...ResourceManager/Resources/Commands.ResourceManager/Cmdlets/Extensions/StringExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.Extensions
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;

/// <summary>
/// String extension methods
Expand Down Expand Up @@ -94,5 +95,24 @@ public static bool IsDateTime(this string source, string format, DateTimeStyles
DateTime parsedDateTime;
return DateTime.TryParseExact(s: source, format: format, provider: null, style: styles, result: out parsedDateTime);
}

/// <summary>
/// Normalize a location string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to provide an explanation, as we discussed offline

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add more details in description.

/// </summary>
/// <param name="location">The location string.</param>
public static string ToNormalizedLocation(this string location)
{
return location == null ? null : new string(location.Where(c => char.IsLetterOrDigit(c)).ToArray());
}

/// <summary>
/// Compare two location strings.
/// </summary>
/// <param name="location1">The first location string.</param>
/// <param name="location2">The second location string.</param>
public static bool EqualsAsLocation(this string location1, string location2)
{
return location1.ToNormalizedLocation().EqualsInsensitively(location2.ToNormalizedLocation());
}
}
}
39 changes: 37 additions & 2 deletions ...urces/Commands.ResourceManager/Cmdlets/Implementation/Providers/GetAzureProviderCmdlet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@

namespace Microsoft.Azure.Commands.ResourceManager.Cmdlets.Implementation
{
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels;
using System.Linq;
using System.Management.Automation;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Extensions;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkExtensions;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels;

/// <summary>
/// Get an existing resource.
Expand Down Expand Up @@ -59,7 +61,7 @@ public class GetAzureProviderCmdlet : ResourceManagerCmdletBase
/// </summary>
public override void ExecuteCmdlet()
{
var providers = this.ResourceManagerSdkClient.ListPSResourceProviders(providerName: this.ProviderNamespace, listAvailable: this.ListAvailable, location: this.Location);
var providers = this.ListPSResourceProviders();

if (!string.IsNullOrEmpty(this.ProviderNamespace))
{
Expand Down Expand Up @@ -89,5 +91,38 @@ public override void ExecuteCmdlet()
this.WriteObject(providers, enumerateCollection: true);
}
}

private PSResourceProvider[] ListPSResourceProviders()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed PS stands for PowerShell, its good to call it ListPowerShellResourceProviders

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add summary

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PowerShell practice is in preference to PS abbreviation. For example, the base class PSObject, PSCustomObject. Here I think we can follow the practice.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PowerShell practice is in preference to PS abbreviation. For example, the base class PSObject, PSCustomObject. Here I think we can follow the practice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good

In reply to: 65991244 [](ancestors = 65991244)

{
var providers = this.ResourceManagerSdkClient.ListResourceProviders(
providerName: this.ProviderNamespace,
listAvailable: this.ListAvailable);

if (string.IsNullOrEmpty(this.Location))
{
return providers
.Select(provider => provider.ToPSResourceProvider())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure this method is introduced in this CR, but good to spell it out here too

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as the other one.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the other one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG

In reply to: 65991290 [](ancestors = 65991290)

.ToArray();
}

var validLocations = this.SubscriptionSdkClient.ListLocations(DefaultContext.Subscription.Id.ToString());

if (!validLocations.Any(loc => loc.Name.EqualsAsLocation(this.Location)))
{
return new PSResourceProvider[] { };
}

foreach (var provider in providers)
{
provider.ResourceTypes = provider.ResourceTypes
.Where(type => !type.Locations.Any() || type.Locations.Any(loc => loc.EqualsAsLocation(this.Location)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check type.Locations not null before type.Locations.Any()

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix this.

.ToList();
}

return providers
.Where(provider => provider.ResourceTypes.Any())
.Select(provider => provider.ToPSResourceProvider())
.ToArray();
}
}
}
12 changes: 6 additions & 6 deletions ...urces/Commands.ResourceManager/Cmdlets/Implementation/Resource/MoveAzureResourceCmdlet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ public class MoveAzureResourceCommand : ResourceManagerCmdletBase
[Alias("Id", "SubscriptionId")]
public Guid? DestinationSubscriptionId { get; set; }

/// <summary>
/// Gets or sets a value that indicates if the user should be prompted for confirmation.
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Do not ask for confirmation.")]
public SwitchParameter Force { get; set; }

/// <summary>
/// Gets or sets the ids of the resources to move.
/// </summary>
Expand All @@ -66,6 +60,12 @@ public class MoveAzureResourceCommand : ResourceManagerCmdletBase
[ValidateNotNullOrEmpty]
public string[] ResourceId { get; set; }

/// <summary>
/// Gets or sets a value that indicates if the user should be prompted for confirmation.
/// </summary>
[Parameter(Mandatory = false, HelpMessage = "Do not ask for confirmation.")]
public SwitchParameter Force { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothings changed, why changing the location in the file?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order of the parameters were changed. This was to fix an auto-populating issue on parameters.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG


/// <summary>
/// Collects subscription ids from the pipeline.
/// </summary>
Expand Down
52 changes: 26 additions & 26 deletions ...ds.ResourceManager/Cmdlets/Microsoft.Azure.Commands.ResourceManager.Cmdlets.format.ps1xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,34 +282,34 @@
</ListControl>
</View>
<View>
<Name>Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels.PSResourceProviderType</Name>
<Name>Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels.PSResourceProvider</Name>
<ViewSelectedBy>
<TypeName>Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels.PSResourceProviderType</TypeName>
<TypeName>Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels.PSResourceProvider</TypeName>
</ViewSelectedBy>
<TableControl>
<TableHeaders>
<TableColumnHeader>
<Label>Name</Label>
<Width>50</Width>
</TableColumnHeader>
<TableColumnHeader>
<Label>Locations</Label>
</TableColumnHeader>
</TableHeaders>
<TableRowEntries>
<TableRowEntry>
<Wrap/>
<TableColumnItems>
<TableColumnItem>
<ScriptBlock>$_.Name</ScriptBlock>
</TableColumnItem>
<TableColumnItem>
<ScriptBlock>$_.LocationsString</ScriptBlock>
</TableColumnItem>
</TableColumnItems>
</TableRowEntry>
</TableRowEntries>
</TableControl>
<ListControl>
<ListEntries>
<ListEntry>
<ListItems>
<ListItem>
<Label>ProviderNamespace</Label>
<PropertyName>ProviderNamespace</PropertyName>
</ListItem>
<ListItem>
<Label>RegistrationState</Label>
<PropertyName>RegistrationState</PropertyName>
</ListItem>
<ListItem>
<Label>ResourceTypes</Label>
<PropertyName>ResourceTypes</PropertyName>
</ListItem>
<ListItem>
<Label>Locations</Label>
<PropertyName>Locations</PropertyName>
</ListItem>
</ListItems>
</ListEntry>
</ListEntries>
</ListControl>
</View>
</ViewDefinitions>
</Configuration>
95 changes: 2 additions & 93 deletions ...eManager/Resources/Commands.ResourceManager/Cmdlets/SdkClient/ResourceManagerSdkClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
using Microsoft.Azure.Commands.Common.Authentication.Models;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Components;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Entities;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Extensions;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkExtensions;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.SdkModels;
using Microsoft.Azure.Commands.ResourceManager.Cmdlets.Utilities;
Expand Down Expand Up @@ -430,30 +431,6 @@ private TemplateValidationInfo CheckBasicDeploymentErrors(string resourceGroup,
return new TemplateValidationInfo(validationResult);
}

public virtual PSResourceProvider[] ListPSResourceProviders(string providerName = null, bool listAvailable = false, string location = null)
{
var providers = this.ListResourceProviders(providerName: providerName, listAvailable: listAvailable);

if (string.IsNullOrEmpty(location))
{
return providers
.Select(provider => provider.ToPSResourceProvider())
.ToArray();
}

foreach (var provider in providers)
{
provider.ResourceTypes = provider.ResourceTypes
.Where(type => !type.Locations.Any() || this.ContainsNormalizedLocation(type.Locations.ToArray(), location))
.ToList();
}

return providers
.Where(provider => provider.ResourceTypes.Any())
.Select(provider => provider.ToPSResourceProvider())
.ToArray();
}

public virtual List<Provider> ListResourceProviders(string providerName = null, bool listAvailable = true)
{
if (!string.IsNullOrEmpty(providerName))
Expand Down Expand Up @@ -485,16 +462,6 @@ public virtual List<Provider> ListResourceProviders(string providerName = null,
}
}

private bool ContainsNormalizedLocation(string[] locations, string location)
{
return locations.Any(existingLocation => this.NormalizeLetterOrDigitToUpperInvariant(existingLocation).Equals(this.NormalizeLetterOrDigitToUpperInvariant(location)));
}

private string NormalizeLetterOrDigitToUpperInvariant(string value)
{
return value != null ? new string(value.Where(c => char.IsLetterOrDigit(c)).ToArray()).ToUpperInvariant() : null;
}

private bool IsProviderRegistered(Provider provider)
{
return string.Equals(
Expand Down Expand Up @@ -540,64 +507,6 @@ public ResourceIdentifier[] ParseResourceIds(string[] resourceIds)
.ToArray();
}

/// <summary>
/// Get a mapping of Resource providers that support the operations API (/operations) to the operations api-version supported for that RP
/// (Current logic is to prefer the latest "non-test' api-version. If there are no such version, choose the latest one)
/// </summary>
public Dictionary<string, string> GetResourceProvidersWithOperationsSupport()
{
PSResourceProvider[] allProviders = this.ListPSResourceProviders(listAvailable: true);

Dictionary<string, string> providersSupportingOperations = new Dictionary<string, string>(StringComparer.InvariantCultureIgnoreCase);
PSResourceProviderResourceType[] providerResourceTypes = null;

foreach (PSResourceProvider provider in allProviders)
{
providerResourceTypes = provider.ResourceTypes;
if (providerResourceTypes != null && providerResourceTypes.Any())
{
PSResourceProviderResourceType operationsResourceType = providerResourceTypes.Where(r => r != null && r.ResourceTypeName == ResourceManagerSdkClient.Operations).FirstOrDefault();
if (operationsResourceType != null &&
operationsResourceType.ApiVersions != null &&
operationsResourceType.ApiVersions.Any())
{
string[] allowedTestPrefixes = new[] { "-preview", "-alpha", "-beta", "-rc", "-privatepreview" };
List<string> nonTestApiVersions = new List<string>();

foreach (string apiVersion in operationsResourceType.ApiVersions)
{
bool isTestApiVersion = false;
foreach (string testPrefix in allowedTestPrefixes)
{
if (apiVersion.EndsWith(testPrefix, StringComparison.InvariantCultureIgnoreCase))
{
isTestApiVersion = true;
break;
}
}

if (isTestApiVersion == false && !nonTestApiVersions.Contains(apiVersion))
{
nonTestApiVersions.Add(apiVersion);
}
}

if (nonTestApiVersions.Any())
{
string latestNonTestApiVersion = nonTestApiVersions.OrderBy(o => o).Last();
providersSupportingOperations.Add(provider.ProviderNamespace, latestNonTestApiVersion);
}
else
{
providersSupportingOperations.Add(provider.ProviderNamespace, operationsResourceType.ApiVersions.OrderBy(o => o).Last());
}
}
}
}

return providersSupportingOperations;
}

/// <summary>
/// Creates a new resource group
/// </summary>
Expand Down Expand Up @@ -667,7 +576,7 @@ public virtual List<PSResourceGroup> FilterResourceGroups(string name, Hashtable
}

resourceGroups = !string.IsNullOrEmpty(location)
? resourceGroups.Where(resourceGroup => this.NormalizeLetterOrDigitToUpperInvariant(resourceGroup.Location).Equals(this.NormalizeLetterOrDigitToUpperInvariant(location))).ToList()
? resourceGroups.Where(resourceGroup => resourceGroup.Location.EqualsAsLocation(location)).ToList()
: resourceGroups;

// TODO: Replace with server side filtering when available
Expand Down
38 changes: 33 additions & 5 deletions ...esourceManager/Resources/Commands.Resources.Test/Providers/GetAzureProviderCmdletTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,31 +49,40 @@ public class GetAzureProviderCmdletTests : RMTestBase
private MockCommandRuntime mockRuntime;

/// <summary>
/// A mock of the client
/// A mock of the IProvidersOperations
/// </summary>
private readonly Mock<IProvidersOperations> providerOperationsMock;

/// <summary>
/// A mock of the ISubscriptionsOperations
/// </summary>
private readonly Mock<ISubscriptionsOperations> subscriptionsOperationsMock;

/// <summary>
/// Initializes a new instance of the <see cref="GetAzureProviderCmdletTests"/> class.
/// </summary>
public GetAzureProviderCmdletTests(ITestOutputHelper output)
{
this.providerOperationsMock = new Mock<IProvidersOperations>();
this.subscriptionsOperationsMock = new Mock<ISubscriptionsOperations>();
XunitTracingInterceptor.AddToContext(new XunitTracingInterceptor(output));
var resourceManagementClient = new Mock<Microsoft.Azure.Management.ResourceManager.IResourceManagementClient>();
var subscriptionClient = new Mock<Microsoft.Azure.Management.ResourceManager.ISubscriptionClient>();

resourceManagementClient
.SetupGet(client => client.Providers)
.Returns(() => this.providerOperationsMock.Object);

subscriptionClient
.SetupGet(client => client.Subscriptions)
.Returns(() => this.subscriptionsOperationsMock.Object);

this.commandRuntimeMock = new Mock<ICommandRuntime>();
this.cmdlet = new GetAzureProviderCmdletTest
{
//CommandRuntime = commandRuntimeMock.Object,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove comment

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix it.

ResourceManagerSdkClient = new ResourceManagerSdkClient
{
ResourceManagementClient = resourceManagementClient.Object,
}
ResourceManagerSdkClient = new ResourceManagerSdkClient(resourceManagementClient.Object),
SubscriptionSdkClient = new SubscriptionSdkClient(subscriptionClient.Object)
};
PSCmdletExtensions.SetCommandRuntimeMock(cmdlet, commandRuntimeMock.Object);
mockRuntime = new MockCommandRuntime();
Expand Down Expand Up @@ -134,6 +143,25 @@ public void GetsResourceProviderTests()
.Setup(f => f.ListWithHttpMessagesAsync(null, null, It.IsAny<CancellationToken>()))
.Returns(() => Task.FromResult(result));

var locationList = new List<Location>
{
new Location
{
Name = "southus",
DisplayName = "South US",
}
};
var pagableLocations = new Page<Location>();
pagableLocations.SetItemValue<Location>(locationList);
var locationsResult = new AzureOperationResponse<IPage<Location>>()
{
Body = pagableLocations
};
this.subscriptionsOperationsMock
.Setup(f => f.ListLocationsWithHttpMessagesAsync(It.IsAny<string>(), null, It.IsAny<CancellationToken>()))
.Returns(() => Task.FromResult(locationsResult));


// 1. List only registered providers
this.commandRuntimeMock
.Setup(m => m.WriteObject(It.IsAny<object>()))
Expand Down
4 changes: 4 additions & 0 deletions src/ResourceManager/Resources/Commands.Resources.Test/ScenarioTests/ProviderTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ function Test-AzureProvider

Assert-True { $allProviders.Length -gt $defaultProviders.Length }

$nonProviders = Get-AzureRmResourceProvider -Location "abc"

Assert-True { $nonProviders.Length -eq 0 }

Register-AzureRmResourceProvider -ProviderName "Microsoft.ApiManagement" -Force

$endTime = [DateTime]::UtcNow.AddMinutes(5)
Expand Down
Loading