#ApplicationGateway Introduce SecureString to cmdlets. #4898

jobatzil · 2017-10-31T22:08:05Z

Description

Change type of -Password parameter of ApplciationGatewaySslCertitificate cmdlets to SecureString.

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.

…cmdlets.

maddieclayton · 2017-10-31T23:33:11Z

@jobatzil in order to fix the build, you need to add the breaking change errors here to the breaking change file here. Let me know if you have any questions or if this doesn't fix your build.

maddieclayton

Some initial comments

maddieclayton · 2017-10-31T23:35:47Z

...mands.Network/ApplicationGateway/SslCertificate/AzureApplicationGatewaySslCertificateBase.cs


        public PSApplicationGatewaySslCertificate NewObject()
        {
            var sslCertificate = new PSApplicationGatewaySslCertificate();

            sslCertificate.Name = this.Name;
-            sslCertificate.Data = Convert.ToBase64String(File.ReadAllBytes(CertificateFile));
+            sslCertificate.Data = Convert.ToBase64String(File.ReadAllBytes(this.CertificateFile));
 #pragma warning disable 0618


You can get rid of the pragma warnings here because it was suppressing the Obsolete warning. Please also get rid of the pragma warnings in Set-AzureRmApplicationGatewaySslCertificate.

maddieclayton · 2017-10-31T23:55:25Z

src/ResourceManager/Network/documentation/current-breaking-changes.md

@@ -38,4 +38,19 @@
    https://github.com/Azure/azure-powershell/blob/dev/documentation/breaking-changes/breaking-change-template.md
 -->

-## Current Breaking Changes


Please revert all changes here - we will automatically move upcoming breaking changes to current breaking changes when we release.

Okay, good to know.

maddieclayton · 2017-10-31T23:55:34Z

src/ResourceManager/Network/documentation/upcoming-breaking-changes.md

@@ -25,18 +25,4 @@
    https://github.com/Azure/azure-powershell/blob/dev/documentation/breaking-changes/breaking-change-template.md
 -->

-# Upcoming Breaking Changes


Same comment

jobatzil · 2017-11-01T03:34:15Z

@maddieclayton Wasn't aware that there is a StaticAnalysis exceptions file for Breaking Changes as well. Good to know, thanks for the comment :)

jobatzil · 2017-11-01T04:06:10Z

I added the entries to the BreakingChangeIssues.csv with the whole path (d:\workspace\...). I saw a lot of entries with equivalent formats. In case this fails I'll change it.

maddieclayton · 2017-11-01T22:07:20Z

@jobatzil Please fix the merge conflict and push again.

maddieclayton · 2017-11-02T00:09:44Z

On-demand here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/112/

jobatzil added 3 commits October 31, 2017 11:22

#AGW added SecureString support for ApplicationGatewaySslCertificate …

d9230b5

…cmdlets.

Added documentation and fixed tests.

b353646

Merge remote-tracking branch 'upstream/preview' into hotfix/secureString

b7cad53

jobatzil changed the title ~~Hotfix/secure string~~ #ApplicationGateway Introduce SecureString to cmdlets. Oct 31, 2017

azuresdkci added the needs-review label Oct 31, 2017

azuresdkci assigned shahabhijeet Oct 31, 2017

maddieclayton assigned maddieclayton and unassigned shahabhijeet Oct 31, 2017

maddieclayton suggested changes Oct 31, 2017

View reviewed changes

maddieclayton assigned jobatzil and unassigned maddieclayton Oct 31, 2017

maddieclayton added needs-revision and removed needs-review labels Oct 31, 2017

Addressed comments.

520618a

Merge remote-tracking branch 'upstream/preview' into hotfix/secureString

a937779

jobatzil requested a review from puneetsaraswat November 1, 2017 17:01

Added certificates to \tools\GlobalFilters.xml

bd8222f

Merge remote-tracking branch 'upstream/preview' into hotfix/secureString

f32a243

maddieclayton added Waiting for On Demand and removed needs-revision labels Nov 2, 2017

cormacpayne approved these changes Nov 2, 2017

View reviewed changes

maddieclayton approved these changes Nov 2, 2017

View reviewed changes

maddieclayton merged commit d273037 into Azure:preview Nov 2, 2017

maddieclayton mentioned this pull request Nov 2, 2017

Password parameter and other password-like parameters expect a plain-text string #4359

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#ApplicationGateway Introduce SecureString to cmdlets. #4898

#ApplicationGateway Introduce SecureString to cmdlets. #4898

jobatzil commented Oct 31, 2017 •

edited by azuresdkci

Loading

maddieclayton commented Oct 31, 2017

maddieclayton left a comment

maddieclayton Oct 31, 2017

jobatzil Nov 1, 2017

maddieclayton Oct 31, 2017

jobatzil Nov 1, 2017

maddieclayton Oct 31, 2017

jobatzil commented Nov 1, 2017

jobatzil commented Nov 1, 2017

maddieclayton commented Nov 1, 2017

maddieclayton commented Nov 2, 2017

#ApplicationGateway Introduce SecureString to cmdlets. #4898

#ApplicationGateway Introduce SecureString to cmdlets. #4898

Conversation

jobatzil commented Oct 31, 2017 • edited by azuresdkci Loading

Description

maddieclayton commented Oct 31, 2017

maddieclayton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jobatzil commented Nov 1, 2017

jobatzil commented Nov 1, 2017

maddieclayton commented Nov 1, 2017

maddieclayton commented Nov 2, 2017

jobatzil commented Oct 31, 2017 •

edited by azuresdkci

Loading