adding support for backing up and restoring of KeyVault secrets #3883

dragav · 2017-05-02T01:49:42Z

adding support for backing up and restoring of KeyVault secrets
revising KeyVault backup/restore cmdlets:
adding support for pipelining
allowing overwriting of dest file
using session data store for file operations

addressing CR feedback: using nameof, discarding redundant parameter set clauses
fix build break following merge
CR feedback and fixing build breaks from merging changes

Description

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.

revising KeyVault backup/restore cmdlets: adding support for pipelining allowing overwriting of dest file using session data store for file operations addressing CR feedback: using nameof, discarding redundant parameter set clauses more CR feedback addressing CR feedback: using nameof, discarding redundant parameter set clauses fix build break following merge CR feedback and fixing build breaks from merging changes adding support for backing up and restoring of KeyVault secrets addressing CR feedback: using nameof, discarding redundant parameter set clauses addressing CR feedback: using nameof, discarding redundant parameter set clauses

msftclas · 2017-05-02T01:49:46Z

@dragav,
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

dragav · 2017-05-02T01:51:05Z

Previous PR

markcowl · 2017-05-02T03:06:21Z

src/ResourceManager/KeyVault/Commands.KeyVault/Commands/BackupAzureKeyVaultKey.cs

+        /// </summary>
+        [Parameter( Mandatory = false,
+                    Position = 3,
+                    ValueFromPipelineByPropertyName = true,


Should not get its value from the pipeline at all

I was following the existing pattern. It looks like you're ok with the current shape of these changes, and so I'll address this with the next opportunity. I have another change in the .. pipeline (no pun intended) for the 5/10 release. Thanks.

yes, that is fine, this is not functionally significant

markcowl · 2017-05-02T03:06:52Z

src/ResourceManager/KeyVault/Commands.KeyVault/Commands/BackupAzureKeyVaultSecret.cs

+        /// </summary>
+        [Parameter( Mandatory = false,
+                    Position = 3,
+                    ValueFromPipelineByPropertyName = true,


Should not get value from pipeline at all

same - ok to leave this for later change

markcowl

Need to update BrekingChangeIssues for any Breaking Changes in this PR

markcowl

Also, need to add breaking change suppressions for your change to BackupAzureKeyVaultKey

markcowl · 2017-05-02T17:39:04Z

src/ResourceManager/KeyVault/Commands.KeyVault/Commands/BackupAzureKeyVaultKey.cs

 using KeyVaultProperties = Microsoft.Azure.Commands.KeyVault.Properties;

 namespace Microsoft.Azure.Commands.KeyVault
 {
    /// <summary>
    /// Requests that a backup of the specified key be downloaded and stored to a file
    /// </summary>
+    /// <remarks>
+    /// The cmdlet returns the path of the newly created backup file.
+    /// </remarks>
    [Cmdlet(VerbsData.Backup, "AzureKeyVaultKey",
        SupportsShouldProcess = true, 
        HelpUri = Constants.KeyVaultHelpUri)]


Need to add a DefaultParameterSetName value, since you now have multiple parameter sets

Thanks, fixed.

setting default parameters for Backup commands

cormacpayne · 2017-05-02T21:29:22Z

@dragav you will need to take the contents of src/Package/BreakingChangeIssues.csv in the Build Artifacts folder from the job associated with the latest commit and paste them into tools/StaticAnalysis/Exceptions/BreakingChangeIssues.csv so that they will be suppressed during the build

…e/BackupRestoreKeyVaultSecrets

markcowl · 2017-05-02T22:50:55Z

tools/StaticAnalysis/Exceptions/BreakingChangeIssues.csv

+"Microsoft.Azure.Commands.KeyVault.dll","Microsoft.Azure.Commands.KeyVault.BackupAzureKeyVaultKey","Backup-AzureKeyVaultKey","0","2100","The parameter 'VaultName' in cmdlet 'Backup-AzureKeyVaultKey' is no longer in the parameter set '__AllParameterSets'.","Add parameter 'VaultName' back to the parameter set '__AllParameterSets'."
+"Microsoft.Azure.Commands.KeyVault.dll","Microsoft.Azure.Commands.KeyVault.BackupAzureKeyVaultKey","Backup-AzureKeyVaultKey","0","2100","The parameter 'Name' in cmdlet 'Backup-AzureKeyVaultKey' is no longer in the parameter set '__AllParameterSets'.","Add parameter 'Name' back to the parameter set '__AllParameterSets'."
+"Microsoft.Azure.Commands.MachineLearning.dll","Microsoft.Azure.Commands.MachineLearning.UpdateAzureMLWebService","Update-AzureRmMlWebService","0","3040","The generic type argument for 'property Parameters' has been changed from 'System.String' to 'Microsoft.Azure.Management.MachineLearning.WebServices.Models.WebServiceParameter'.","Change the generic type argument for 'property Parameters' back to 'System.String'."


This last entry is repeated, but we don't need to change it right now.

Removing duplicate value and extra lines

markcowl · 2017-05-02T23:39:51Z

@azuresdkci test this please

cormacpayne · 2017-05-03T00:51:57Z

On-demand: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1525/

msftclas added the cla-not-required label May 2, 2017

azuresdkci assigned cormacpayne May 2, 2017

markcowl approved these changes May 2, 2017

View reviewed changes

azuresdkci added the needs-review label May 2, 2017

cormacpayne removed the needs-review label May 2, 2017

markcowl requested changes May 2, 2017

View reviewed changes

updating change and breaking-change logs

5939c53

setting default parameters for Backup commands

cormacpayne removed their assignment May 2, 2017

cormacpayne added the needs-revision label May 2, 2017

Dragos Avadanei and others added 3 commits May 2, 2017 14:37

Merge remote-tracking branch 'refs/remotes/Azure/preview' into featur…

bcce45a

…e/BackupRestoreKeyVaultSecrets

adding suppression for breaking change analysis

e46a67d

Merge branch 'preview' into feature/BackupRestoreKeyVaultSecrets

342bed3

markcowl reviewed May 2, 2017

View reviewed changes

markcowl previously approved these changes May 2, 2017

View reviewed changes

Updating braking change suppressions

0e48389

Removing duplicate value and extra lines

markcowl dismissed their stale review via 0e48389 May 2, 2017 23:35

markcowl approved these changes May 2, 2017

View reviewed changes

cormacpayne added Waiting for On Demand and removed needs-revision labels May 3, 2017

markcowl merged commit b28bfd4 into Azure:preview May 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding support for backing up and restoring of KeyVault secrets #3883

adding support for backing up and restoring of KeyVault secrets #3883

dragav commented May 2, 2017 •

edited

Loading

msftclas commented May 2, 2017

dragav commented May 2, 2017

markcowl May 2, 2017

dragav May 2, 2017

markcowl May 2, 2017

markcowl May 2, 2017

markcowl May 2, 2017

markcowl left a comment

markcowl left a comment

markcowl May 2, 2017

dragav May 2, 2017

cormacpayne commented May 2, 2017

markcowl May 2, 2017

markcowl commented May 2, 2017

cormacpayne commented May 3, 2017

adding support for backing up and restoring of KeyVault secrets #3883

adding support for backing up and restoring of KeyVault secrets #3883

Conversation

dragav commented May 2, 2017 • edited Loading

Description

msftclas commented May 2, 2017

dragav commented May 2, 2017

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

markcowl left a comment

Choose a reason for hiding this comment

markcowl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cormacpayne commented May 2, 2017

Choose a reason for hiding this comment

markcowl commented May 2, 2017

cormacpayne commented May 3, 2017

dragav commented May 2, 2017 •

edited

Loading