#WAF Adding WAF support to PowerShell by jobatzil · Pull Request #3678 · Azure/azure-powershell

jobatzil · 2017-03-28T16:51:12Z

Description

Add support of newest WAF functionality to PowerShell.

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

…convertion error, removed shouldProcess from WAF, enhanced WAF/ConnectionDraining Set... cmdlets.

msftclas · 2017-03-28T16:51:17Z

@jobatzil,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

cormacpayne

@jobatzil Hey Johannes, a couple of comments that need to be resolved

cormacpayne · 2017-03-28T18:30:10Z

-    <Reference Include="Microsoft.Azure.Management.Network, Version=9.1.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
-      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Network.9.1.0-preview\lib\net45\Microsoft.Azure.Management.Network.dll</HintPath>
+    <Reference Include="Microsoft.Azure.Management.Network, Version=9.2.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\..\packages\Microsoft.Azure.Management.Network.9.2.0-preview\lib\net45\Microsoft.Azure.Management.Network.dll</HintPath>


@jobatzil 10.0.0-preview

cormacpayne · 2017-03-28T18:30:28Z

@@ -7,7 +7,7 @@
  <package id="Microsoft.Azure.Graph.RBAC" version="3.2.0-preview" targetFramework="net45" />
  <package id="Microsoft.Azure.Management.Authorization" version="1.0.0" targetFramework="net45" />
  <package id="Microsoft.Azure.Management.Compute" version="14.0.0-prerelease" targetFramework="net45" />
-  <package id="Microsoft.Azure.Management.Network" version="9.1.0-preview" targetFramework="net45" />
+  <package id="Microsoft.Azure.Management.Network" version="9.2.0-preview" targetFramework="net45" />


@jobatzil 10.0.0-preview

cormacpayne · 2017-03-28T18:32:59Z

+               Mandatory = true,
+               HelpMessage = "The version of the rule set type.")]
+        [ValidateNotNullOrEmpty]
+        public string RuleSetVersion { get; set; }


@jobatzil adding parameters RuleSetType and RuleSetVersion that are mandatory is a breaking change - existing scripts that use these cmdlets will not work

Yes, it is also mandatory in the new azure-sdk

cormacpayne · 2017-03-28T18:33:30Z

@@ -17,24 +17,24 @@

 namespace Microsoft.Azure.Commands.Network
 {
-    [Cmdlet(VerbsCommon.New, "AzureRmApplicationGatewayWebApplicationFirewallConfiguration", SupportsShouldProcess = true),


@jobatzil why was this removed?

it only changes the local object, it shouldn't have been there in the first place.

cormacpayne · 2017-03-28T18:33:38Z

@@ -17,7 +17,7 @@

 namespace Microsoft.Azure.Commands.Network
 {
-    [Cmdlet(VerbsCommon.Set, "AzureRmApplicationGatewayWebApplicationFirewallConfiguration", SupportsShouldProcess = true),


@jobatzil why was this removed?

same as above.

cormacpayne · 2017-03-28T18:36:35Z

+        [Parameter(
+            Mandatory = false,
+            HelpMessage = "The list of rules that will be disabled. If null, all rules of the rule group will be disabled.")]
+        public List<int> Rules { get; set; }


@jobatzil according to PowerShell Strongly Encouraged Development Guidelines, parameter nouns should be singular:

Avoid using plural names for parameters whose value is a single element. This includes parameters that take arrays or lists because the user might supply an array or list with only one element.

Let's think of a different noun that can be used instead of Rules. How does RuleList work?

Hmm, this is the name we use on every level (powershell, azure-ask, backend)

I will leave it like that to keep it consistent.

cormacpayne · 2017-03-28T18:37:29Z

+
+namespace Microsoft.Azure.Commands.Network
+{
+    [Cmdlet(VerbsCommon.New, "AzureRmApplicationGatewayFirewallDisabledRuleGroupConfig"), 


@jobatzil I'm worried that the name of this cmdlet is too long. Is there any way we can keep the name, but add an alias to something shorter?

As discussed offline, there is no shorter name that still matches our current pattern. We are actually thinking of introducing shorter aliases for all of the cmdlets (agw instead of applicationGateway?), this will not be part of this PR though.

cormacpayne · 2017-03-28T18:39:37Z

+        public List<PSApplicationGatewayFirewallRuleSet> Value { get; set; }
+
+        [JsonIgnore]
+        public string ValueText


@jobatzil for each of the classes you are adding to the Model folder, can you make sure that each of their properties have corresponding help with them?

/// <summary> /// Brief summary of property. /// </summary> public string ValueText { ... }

We've never done that before, but I can do that.

cormacpayne · 2017-03-28T18:41:20Z

@@ -30,6 +30,13 @@ public ApplicationGatewayTests(ITestOutputHelper output)

        [Fact]
        [Trait(Category.AcceptanceType, Category.CheckIn)]
+        public void TestAvailableWafRuleSets()
+        {
+            NetworkResourcesController.NewInstance.RunPsTest(string.Format("Test-AvailableWafRuleSets -baseDir '{0}'", AppDomain.CurrentDomain.BaseDirectory));


@jobatzil was there a test recording file associated with this test? I didn't see one. If so, make sure it is added in the test csproj file.

yes, but it is not uploaded yet, I may change the cmdlet before it gets finalized.

I uploaded the test recordings.

…ilableWafRuleSets test recordings and and alias. Added StartUp script for network.

…eSets to the notes section in the markdown file.

jobatzil · 2017-03-30T00:52:33Z

@cormacpayne I didn't update the TestApplicationGatewayCRUD test recordings yet.

…aking changes.

…j file.

…sion.

jobatzil added 11 commits March 20, 2017 20:27

#WAF Added reference to own signed azure-sdk-for-net package.

56098a1

#WAF changed version in rest API calls of Network/SessionRecords.

43ebbfe

#WAF changed version in rest API calls of Compute/SessionRecords.

c5fb86d

#WAF Added object model for waf.

0d8facb

#WAF Updated/Added cmdlets.

deb4cd0

#WAF Fixed documentation error for set-Azure...ProbeConfig

6528f98

#WAF updated .csproj file

63533e5

#WAF changed disabledRuleGroup cmdlet name, fixed null to empty list …

3800945

…convertion error, removed shouldProcess from WAF, enhanced WAF/ConnectionDraining Set... cmdlets.

#WAF Added first version of documentation.

198d41b

#WAF updated reference to new network version (10.0.0-preview.

0b3b872

#WAF fixed formatting in test and field name.

4e32ae6

msftclas added the cla-not-required label Mar 28, 2017

azuresdkci assigned shahabhijeet Mar 28, 2017

cormacpayne suggested changes Mar 28, 2017

View reviewed changes

cormacpayne assigned jobatzil and unassigned shahabhijeet Mar 28, 2017

cormacpayne added the needs-revision label Mar 28, 2017

jobatzil added 4 commits March 29, 2017 17:01

#WAF Added ValidateSet and DefaultValue for new/set-WAF. Added getAva…

cd44126

…ilableWafRuleSets test recordings and and alias. Added StartUp script for network.

#WAF Added ShouldProcess suppresions, updated markdown files.

7eca34f

Merge remote-tracking branch 'upstream/dev' into feature/jobatzil/waf

edaaedc

#WAF Added the alias for Get-AzureRmApplicationGatewayAvailableWafRul…

7218db7

…eSets to the notes section in the markdown file.

jobatzil added 7 commits March 29, 2017 22:26

#WAF Fixed and recorded applicationGatewayCRUD test.

65aa4a2

#WAF removed app.config and updated changelog.

e5a500a

#WAF improved naming in NormalizeStringLength method.

705db4a

#WAF Readded ShouldProcess to new/set-WAF cmdlet for now to avoid bre…

dffaba7

…aking changes.

#WAF Updated markdown files.

f7496d8

#WAF removed unneccessary file changes.

439f5d5

#WAF added new lines after `yaml in new/set-WAF cmdlets

07a909f

vinatara approved these changes Mar 30, 2017

View reviewed changes

jobatzil mentioned this pull request Mar 30, 2017

Ipsec Policy for Virtual Network Gateway Connection #3666

Merged

13 tasks

jobatzil added 4 commits March 31, 2017 11:23

#WAF Updated help.xml file

190712c

#WAF fixed missing <modl:para> entries in help.xml

df3ebf9

#WAF added the AvailableWafRuleSets test recording file to the .cspro…

1405bae

…j file.

#WAF updated restrecording of new compute test to new network-api ver…

9ed9c61

…sion.

cormacpayne approved these changes Mar 31, 2017

View reviewed changes

cormacpayne merged commit bc5c27c into Azure:dev Mar 31, 2017

Conversation

jobatzil commented Mar 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

msftclas commented Mar 28, 2017

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jobatzil Mar 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jobatzil commented Mar 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jobatzil commented Mar 28, 2017 •

edited

Loading

jobatzil Mar 28, 2017 •

edited

Loading