Skip to content

Websites: Adding support for MSI & Https in slots #5986

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 4 commits into from
Apr 27, 2018
Merged

Websites: Adding support for MSI & Https in slots #5986

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a0a4a4f
Adding support for MSI & Https in Slots.
panchagnula Apr 19, 2018
5a92009
Adding tests, updating help
panchagnula Apr 20, 2018
87317a8
Fixing build
panchagnula Apr 26, 2018
a0a914a
Updating the test for set slot & updating the session record
panchagnula Apr 26, 2018
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
1 change: 0 additions & 1 deletion src/ResourceManager/Websites/Commands.Websites.Test/Commands.Websites.Test.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
</ItemGroup>
<ItemGroup />
<Import Project="..\..\..\packages\Microsoft.Bcl.Build.1.0.14\tools\Microsoft.Bcl.Build.targets" Condition="Exists('..\..\..\packages\Microsoft.Bcl.Build.1.0.14\tools\Microsoft.Bcl.Build.targets')" />
<Import Project="..\..\..\..\tools\Common.Dependencies.Test.targets" />
<Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
</Project>
7 changes: 5 additions & 2 deletions src/ResourceManager/Websites/Commands.Websites.Test/ScenarioTests/WebAppSlotTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -525,15 +525,18 @@ function Test-SetWebAppSlot
# Assert
Assert-AreEqual $appWithSlotName $slot.Name
Assert-AreEqual $serverFarm1.Id $slot.ServerFarmId
Assert-Null $webApp.Identity

# Change service plan
$job = Set-AzureRmWebAppSlot -ResourceGroupName $rgname -Name $appname -Slot $slotname -AppServicePlan $planName2 -AsJob
# Change service plan & set properties
$job = Set-AzureRmWebAppSlot -ResourceGroupName $rgname -Name $appname -Slot $slotname -AppServicePlan $planName2 -HttpsOnly $true -AssignIdentity $true -AsJob
$job | Wait-Job
$slot = $job | Receive-Job

# Assert
Assert-AreEqual $appWithSlotName $slot.Name
Assert-AreEqual $serverFarm2.Id $slot.ServerFarmId
Assert-AreEqual $true $slot.HttpsOnly
Assert-NotNull $slot.Identity

# Set config properties
$slot.SiteConfig.HttpLoggingEnabled = $true
Expand Down
8 changes: 6 additions & 2 deletions src/ResourceManager/Websites/Commands.Websites.Test/ScenarioTests/WebAppTests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -595,20 +595,24 @@ function Test-SetWebApp
# Assert
Assert-AreEqual $webAppName $webApp.Name
Assert-AreEqual $serverFarm1.Id $webApp.ServerFarmId
Assert-Null $webApp.Identity

# Change service plan
$job = Set-AzureRmWebApp -ResourceGroupName $rgname -Name $webAppName -AppServicePlan $appServicePlanName2 -AsJob
# Change service plan & set site properties
$job = Set-AzureRmWebApp -ResourceGroupName $rgname -Name $webAppName -AppServicePlan $appServicePlanName2 -HttpsOnly $true -AssignIdentity $true -AsJob
$job | Wait-Job
$webApp = $job | Receive-Job

# Assert
Assert-AreEqual $webAppName $webApp.Name
Assert-AreEqual $serverFarm2.Id $webApp.ServerFarmId
Assert-AreEqual $true $webApp.HttpsOnly
Assert-NotNull $webApp.Identity

# Set config properties
$webapp.SiteConfig.HttpLoggingEnabled = $true
$webapp.SiteConfig.RequestTracingEnabled = $true

# Set site properties
$webApp = $webApp | Set-AzureRmWebApp

# Assert
Expand Down
1,426 changes: 869 additions & 557 deletions ...crosoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppSlotTests/TestSetWebAppSlot.json
View file
Open in desktop

Large diffs are not rendered by default.

1,277 changes: 766 additions & 511 deletions ...cords/Microsoft.Azure.Commands.Websites.Test.ScenarioTests.WebAppTests/TestSetWebApp.json
View file
Open in desktop

Large diffs are not rendered by default.

38 changes: 37 additions & 1 deletion src/ResourceManager/Websites/Commands.Websites/Cmdlets/DeploymentSlots/SetAzureWebAppSlot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,17 @@ public class SetAzureWebAppSlotCmdlet : WebAppSlotBaseCmdlet
[Parameter(Mandatory = false, HelpMessage = "Run cmdlet in the background")]
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 tests for these new parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it for the webapp version

public SwitchParameter AsJob { get; set; }

[Parameter(ParameterSetName = ParameterSet1Name, Mandatory = false, HelpMessage = "Enable MSI on an existing azure webapp")]
Copy link
Member

Choose a reason for hiding this comment

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

Is this a PATCH or a PUT? If it is a PATCH, this is OK. If it is a PUT, then these should be switch parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a PATCH Mark, but during our commandlet review (https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/51)we concluded that

  1. this can be confusing for customers, since the rest of the properties use a bool
  2. customers can accidentally turn off the property by running the set command without any parameters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can i propose that we update all the properties of the SetWebApp & SetWebAppSlot to use the switch parameter as a separate change, i agree switch parameter is the valid PowerShell way , however to have consistency we should change all which i want to avoid this time, since its a bigger change. Let me know your thoughts!

Copy link
Member

@markcowl markcowl Apr 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

For PATCH boolean parameters are acceptable, for PUT they are not. I am OK with this for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you this is the issue to track this #6066

public bool AssignIdentity { get; set; }

[Parameter(ParameterSetName = ParameterSet1Name, Mandatory = false, HelpMessage = "Enable/disable redirecting all traffic to HTTPS on an existing azure webapp")]
public bool HttpsOnly { get; set; }

public override void ExecuteCmdlet()
{
base.ExecuteCmdlet();
SiteConfig siteConfig = null;
Site site = null;
string location = null;
switch (ParameterSetName)
{
Expand Down Expand Up @@ -128,12 +135,41 @@ public override void ExecuteCmdlet()
// Update web app configuration
WebsitesClient.UpdateWebAppConfiguration(ResourceGroupName, location, Name, Slot, siteConfig, AppSettings.ConvertToStringDictionary(), ConnectionStrings.ConvertToConnectionStringDictionary());

if (parameters.Any(p => CmdletHelpers.SiteParameters.Contains(p)))
{

site = new Site
{
Location = location,
ServerFarmId = WebApp.ServerFarmId,
Identity = parameters.Contains("AssignIdentity") && AssignIdentity ? new ManagedServiceIdentity("SystemAssigned", null, null) : WebApp.Identity,
HttpsOnly = parameters.Contains("HttpsOnly") ? HttpsOnly : WebApp.HttpsOnly
};

Dictionary<string, string> appSettings = WebApp.SiteConfig?.AppSettings?.ToDictionary(x => x.Name, x => x.Value);
if (parameters.Contains("AssignIdentity"))
{

// Add or update the appsettings property
appSettings["WEBSITE_DISABLE_MSI"] = (!AssignIdentity).ToString();
WebsitesClient.UpdateWebAppConfiguration(ResourceGroupName, location, Name, Slot, WebApp.SiteConfig, appSettings,
WebApp.SiteConfig.ConnectionStrings.
ToDictionary(nvp => nvp.Name,
nvp => new ConnStringValueTypePair
{
Type = nvp.Type.Value,
Value = nvp.ConnectionString
},
StringComparer.OrdinalIgnoreCase));
}
WebsitesClient.UpdateWebApp(ResourceGroupName, location, Name, Slot, WebApp.ServerFarmId, site);
}

if (parameters.Contains("AppServicePlan"))
{
WebsitesClient.UpdateWebApp(ResourceGroupName, location, Name, Slot, AppServicePlan);
}


break;
case ParameterSet2Name:
// Web app is direct or pipeline input
Expand Down
19 changes: 7 additions & 12 deletions src/ResourceManager/Websites/Commands.Websites/Cmdlets/WebApps/SetAzureWebApp.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ public class SetAzureWebAppCmdlet : WebAppBaseCmdlet
public SwitchParameter AsJob { get; set; }

[Parameter(ParameterSetName = ParameterSet1Name, Mandatory = false, HelpMessage = "Enable MSI on an existing azure webapp")]
public SwitchParameter AssignIdentity { get; set; }
public bool AssignIdentity { get; set; }

[Parameter(ParameterSetName = ParameterSet1Name, Mandatory = false, HelpMessage = "Enable/disable redirecting all traffic to HTTPS on an existing azure webapp")]
public SwitchParameter HttpsOnly { get; set; }
public bool HttpsOnly { get; set; }

public override void ExecuteCmdlet()
{
Expand Down Expand Up @@ -145,21 +145,22 @@ public override void ExecuteCmdlet()

if (parameters.Any(p => CmdletHelpers.SiteParameters.Contains(p)))
{

site = new Site
{
Location = location,
ServerFarmId = WebApp.ServerFarmId,
Identity = AssignIdentity.IsPresent ? new ManagedServiceIdentity("SystemAssigned", null, null) : WebApp.Identity,
HttpsOnly = HttpsOnly.IsPresent
Identity = parameters.Contains("AssignIdentity") && AssignIdentity ? new ManagedServiceIdentity("SystemAssigned", null, null) : WebApp.Identity,
HttpsOnly = parameters.Contains("HttpsOnly") ? HttpsOnly : WebApp.HttpsOnly
};

Dictionary<string, string> appSettings = WebApp.SiteConfig?.AppSettings?.ToDictionary(x => x.Name, x => x.Value);
if (AssignIdentity.IsPresent)
if (parameters.Contains("AssignIdentity"))
{

// Add or update the appsettings property
appSettings["WEBSITE_DISABLE_MSI"] = (!AssignIdentity).ToString();
WebsitesClient.UpdateWebAppConfiguration(ResourceGroupName, location, Name, null, WebApp.SiteConfig, appSettings,
WebsitesClient.UpdateWebAppConfiguration(ResourceGroupName, location, Name, null, WebApp.SiteConfig, appSettings,
WebApp.SiteConfig.ConnectionStrings.
ToDictionary(nvp => nvp.Name,
nvp => new ConnStringValueTypePair
Expand All @@ -169,12 +170,6 @@ public override void ExecuteCmdlet()
},
StringComparer.OrdinalIgnoreCase));
}
else
{
// disabling the identity property updates the siteconfig only
appSettings["WEBSITE_DISABLE_MSI"] = (!AssignIdentity.IsPresent).ToString();
}

WebsitesClient.UpdateWebApp(ResourceGroupName, location, Name, null, WebApp.ServerFarmId, site);
}

Expand Down
10 changes: 5 additions & 5 deletions src/ResourceManager/Websites/Commands.Websites/help/Set-AzureRmWebApp.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ Set-AzureRmWebApp [[-AppServicePlan] <String>] [[-DefaultDocuments] <String[]>]
[[-ConnectionStrings] <Hashtable>]
[[-HandlerMappings] <System.Collections.Generic.IList`1[Microsoft.Azure.Management.WebSites.Models.HandlerMapping]>]
[[-ManagedPipelineMode] <String>] [[-WebSocketsEnabled] <Boolean>] [[-Use32BitWorkerProcess] <Boolean>]
[[-AutoSwapSlotName] <String>] [-HostNames <String[]>] [-NumberOfWorkers <Int32>] [-AsJob] [-AssignIdentity]
[-HttpsOnly] [-ResourceGroupName] <String> [-Name] <String> [-DefaultProfile <IAzureContextContainer>]
[[-AutoSwapSlotName] <String>] [-HostNames <String[]>] [-NumberOfWorkers <Int32>] [-AsJob] [[-AssignIdentity] <Boolean>]
[[-HttpsOnly] <Boolean>] [-ResourceGroupName] <String> [-Name] <String> [-DefaultProfile <IAzureContextContainer>]
[<CommonParameters>]
```

Expand Down Expand Up @@ -93,10 +93,10 @@ Accept wildcard characters: False
```

### -AssignIdentity
Enable MSI on an existing azure webapp or functionapp
Enable/disable MSI on an existing azure webapp or functionapp [PREVIEW]

```yaml
Type: SwitchParameter
Type: Boolean
Parameter Sets: S1
Aliases:

Expand Down Expand Up @@ -231,7 +231,7 @@ Accept wildcard characters: False
Enable/disable redirecting all traffic to HTTPS on an existing azure webapp or functionapp

```yaml
Type: SwitchParameter
Type: Boolean
Parameter Sets: S1
Aliases:

Expand Down
31 changes: 30 additions & 1 deletion src/ResourceManager/Websites/Commands.Websites/help/Set-AzureRmWebAppSlot.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Set-AzureRmWebAppSlot [[-AppServicePlan] <String>] [[-DefaultDocuments] <String[
[[-HandlerMappings] <System.Collections.Generic.IList`1[Microsoft.Azure.Management.WebSites.Models.HandlerMapping]>]
[[-ManagedPipelineMode] <String>] [[-WebSocketsEnabled] <Boolean>] [[-Use32BitWorkerProcess] <Boolean>]
[-AutoSwapSlotName <String>] [-NumberOfWorkers <Int32>] [-ResourceGroupName] <String> [-Name] <String>
[-Slot] <String> [-AsJob]
[[-AssignIdentity] <Boolean>] [[HttpsOnly] <Boolean>] [-Slot] <String> [-AsJob]
[-DefaultProfile <IAzureContextContainer>] [<CommonParameters>]
```

Expand Down Expand Up @@ -352,6 +352,35 @@ Default value: None
Accept pipeline input: False
Accept wildcard characters: False
```
### -HttpsOnly
Enable/disable redirecting all traffic to HTTPS on an existing slot

```yaml
Type: Boolean
Parameter Sets: S1
Aliases:

Required: False
Position: Named
Default value: None
Accept pipeline input: False
Accept wildcard characters: False
```
### -AssignIdentity
Enable/disable MSI on an existing slot [PREVIEW]

```yaml
Type: Boolean
Parameter Sets: S1
Aliases:

Required: False
Position: Named
Default value: None
Accept pipeline input: False
Accept wildcard characters: False
```

### -AsJob
Run cmdlet in the background

Expand Down