-
Notifications
You must be signed in to change notification settings - Fork 4k
Issue #16094 - Updated New-AzAppServicePlan to create an app service plan with host environment id. #16645
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.
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
Conversation
ff361eb
to
a8af780
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - only a few changes needed.
@@ -52,6 +52,9 @@ public static class CmdletHelpers | |||
private static readonly Regex KeyVaultResourceIdRegex = | |||
new Regex(@"^\/subscriptions\/(?<subscriptionName>[^\/]+)\/resourceGroups\/(?<resourceGroupName>[^\/]+)\/providers\/Microsoft.KeyVault\/vaults\/(?<vaultName>[^\/]+)$", RegexOptions.IgnoreCase); | |||
|
|||
private static readonly Regex HostingEnvironmentResourceIdRegex = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets keep the AppServiceEnvironment naming consistent instead of HostingEnvironment (of course except when needed for ARM naming)
@@ -269,6 +272,24 @@ internal static string BuildMetricFilter(DateTime? startTime, DateTime? endTime, | |||
return false; | |||
} | |||
|
|||
internal static bool TryParseHostingEnvironmentMetadataFromResourceId(string resourceId, out string resourceGroupName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change method name
internal static bool TryParseHostingEnvironmentMetadataFromResourceId(string resourceId, out string resourceGroupName, | ||
out string aseName) | ||
{ | ||
var match = AppServicePlanResourceIdRegex.Match(resourceId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong RegEx object, so it actually never succeeds.
public SwitchParameter Linux { get; set; } | ||
|
||
public override void ExecuteCmdlet() | ||
{ | ||
if (HyperV.IsPresent && | ||
if (HyperV.IsPresent && | ||
(Tier != "PremiumContainer" && Tier != "PremiumV3")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HyperV is also allowed for ASEv3, so please add: && Tier != "IsolatedV2"
and modify the exception text.
a8af780
to
74970df
Compare
74970df
to
8f55247
Compare
…ith host environment id (Azure#16094)
…e plan with host environment id (Azure#16094)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me - worked with Sam directly on this change.
/azp run azure-powershell - security-tools |
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Checklist
CONTRIBUTING.md
ChangeLog.md
file(s) has been updated:ChangeLog.md
file can be found atsrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
## Upcoming Release
header -- no new version header should be added