Skip to content
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

Support Advanced Threat Protection policy management #8523

Merged
merged 12 commits into from
Feb 15, 2019
Merged

Support Advanced Threat Protection policy management #8523

merged 12 commits into from
Feb 15, 2019

Conversation

shblum
Copy link
Contributor

@shblum shblum commented Feb 12, 2019

Description

Adding Get/Set Advanced Threat Protection cmdlets + Tests + Help files.

Design Review: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/237

Checklist

@adxsdkps
Copy link
Collaborator

Can one of the admins verify this patch?

@maddieclayton
Copy link
Contributor

@azuresdkci Add to whitelist

@maddieclayton
Copy link
Contributor

@shblum Please link existing design review or fill out a new design review here: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues

@cormacpayne
Copy link
Member

Cmdlet review: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/237

@shblum
Copy link
Contributor Author

shblum commented Feb 13, 2019

Design review link is in the description.

@maddieclayton
Copy link
Contributor

@shblum
Copy link
Contributor Author

shblum commented Feb 14, 2019

@maddieclayton maddieclayton self-assigned this Feb 14, 2019
function Get-AdvancedThreatProtectionTestEnvironmentParameters ($testSuffix)
{
return @{ subscriptionId = (Get-AzContext).Subscription.Id;
rgName = "storage-atp-cmdlet-test-rg" +$testSuffix;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use "getAssetName" to add an element of randomness to your resource name generation.

rgName = "storage-atp-cmdlet-test-rg" +$testSuffix;
accountName = "storage" +$testSuffix;
storageSku = "Standard_GRS";
location = "West Central US"
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use GetLocation -ProviderNamespace <namespace> -ResourceType <resourcetype> -PreferredLocation "West Central US" to future proof the recording process.

@@ -63,21 +63,33 @@ public void RunPowerShellTest(ServiceManagement.Common.Models.XunitTracingInterc
_helper.RMProfileModule,
_helper.GetRMModulePath(@"AzureRM.Security.psd1"),
"ScenarioTests\\Common.ps1",
"ScenarioTests\\" + callingClassName + ".ps1");
"ScenarioTests\\" + callingClassName + ".ps1",
_helper.GetRMModulePath(@"AzureRM.Resources"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of using the Resources and Storage modules, please use our internal version of these cmdlets. This is just changing this import to the two ps1 files:

, and updating the client to the common version:
using Microsoft.Azure.Management.Storage.Version2017_10_01;
,
using Microsoft.Azure.Management.Internal.Resources;

@@ -11,7 +11,8 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Azure.Management.SecurityCenter" Version="0.10.0-preview" />
<PackageReference Include="Microsoft.Azure.Management.SecurityCenter" Version="0.11.0-preview" />
<PackageReference Include="Microsoft.Azure.Management.Storage" Version="9.1.0-preview" />
Copy link
Contributor

Choose a reason for hiding this comment

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

You can remove this once you update to the common client.

@@ -41,6 +41,9 @@ Gets the pricing tier data for Azure Security Center for a scope.
### [Get-AzSecurityTask](Get-AzSecurityTask.md)
Gets the security tasks that Azure Security Center recommends you to do in order to strengthen your security posture.

### [Get-AzSecurityThreatProtection](Get-AzSecurityThreatProtection.md)
{{Fill in the Synopsis}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please copy over the synopsis here for the two new cmdlets.

```

## DESCRIPTION
The "Get-AzSecurityThreatProtection" cmdlet gets the threat protetion policy of a storage account.
Copy link
Contributor

Choose a reason for hiding this comment

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

*protection


### Example 1
```powershell
PS C:\> Get-AzSecurityThreatProtection -ResourceId "/subscriptions/cma24pc8-89b5-4aa7-9ff6-486e886c304a/resourceGroups/myResourceGroup/providers/Microsoft.Storage/storageAccounts/myStorageAccount/"
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 example output for all examples

Copy link
Contributor

Choose a reason for hiding this comment

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

Please also remove the subscription and replace with xxxxxxx-xxxx-xxxx-xxxxxxxxxxxxx

PS C:\> Get-AzSecurityThreatProtection -ResourceId "/subscriptions/cma24pc8-89b5-4aa7-9ff6-486e886c304a/resourceGroups/myResourceGroup/providers/Microsoft.Storage/storageAccounts/myStorageAccount/"
```

This command gets the threat protection policy for resource id "/subscriptions/cma24pc8-89b5-4aa7-9ff6-486e886c304a/resourceGroups/myResourceGroup/providers/Microsoft.Storage/storageAccounts/myStorageAccount/".
Copy link
Contributor

Choose a reason for hiding this comment

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

xxxxxxx-xxxx-xxxx-xxxxxxxxxxxxx for subscription

@maddieclayton
Copy link
Contributor

Just to reiterate Cormac's comments in the design review, this design is fine for a preview release, but will need to be revisited before the stable release as it doesn't match our standards for stable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants