Adding new Powershell module for ManagedServiceIdentity #5270

varunkch · 2018-01-12T20:32:13Z

Description

Adding new Powershell module for MSI (Managed Service Identity).

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.

varunkch · 2018-01-12T20:51:25Z

I ran the tests by setting the credentials using Set-TestEnvironment and the tests succeeded but no SessionRecords were generated.

cormacpayne · 2018-01-30T17:52:40Z

@varunkch the build is failing because there is no markdown help provided for your cmdlets. See this guide for more information on using the platyPS module to generate the markdown files for your cmdlets.

maddieclayton

Here is an initial review - I may have a few more comments once these comments are addressed and I can do another in depth review.

maddieclayton · 2018-02-06T21:25:58Z

...sourceManager/Common/Commands.ScenarioTests.ResourceManager.Common/EnvironmentSetupHelper.cs

Is there a reason you can't use GetRMModulePath below? No reason to add a method if it is not necessary.

Changed to use GetRMModulePath

maddieclayton · 2018-02-06T21:26:56Z

src/ResourceManager/ManagedServiceIdentity/AzureRM.ManagedServiceIdentity.psd1

It is standard for modules to start at version 1.0.0.

maddieclayton · 2018-02-06T21:27:53Z

src/ResourceManager/ManagedServiceIdentity/AzureRM.ManagedServiceIdentity.psd1

Please update this to the current version of Profile (4.2.0).

maddieclayton · 2018-02-06T21:30:41Z

src/ResourceManager/ManagedServiceIdentity/AzureRM.ManagedServiceIdentity.psd1

You will need to regenerate the WIX file: https://github.com/Azure/azure-powershell/wiki/Azure-Powershell-Developer-Guide#updating-the-installer

This should fix your build issues.

maddieclayton · 2018-02-06T21:31:33Z

...eIdentity/Commands.ManagedServiceIdentity.Tests/Commands.ManagedServiceIdentity.Tests.csproj

Please change RootNamespace and AssemblyName to Microsoft.Azure.Commands.ManagedServiceIdentity.Tests

maddieclayton · 2018-02-06T23:46:44Z

...nds.ManagedServiceIdentity/UserAssignedIdentities/RemoveAzureRmUserAssignedIdentityCmdlet.cs

Add better message and store in Resources

maddieclayton · 2018-02-06T23:49:20Z

...anagedServiceIdentity/Commands.ManagedServiceIdentity/help/AzureRM.ManagedServiceIdentity.md

Please add descriptions for each of these cmdlets - if you regenerate the description for the individual modules should be automatically filled in, but you will have to fill the module description manually.

maddieclayton · 2018-02-06T23:49:50Z

...nagedServiceIdentity/Commands.ManagedServiceIdentity/help/Get-AzureRmUserAssignedIdentity.md

Please add sample output for all examples.

maddieclayton · 2018-02-06T23:51:25Z

...edServiceIdentity/Commands.ManagedServiceIdentity/help/Remove-AzureRmUserAssignedIdentity.md

Change to System.Boolean

maddieclayton · 2018-02-06T23:53:29Z

...nagedServiceIdentity/Commands.ManagedServiceIdentity/help/Get-AzureRmUserAssignedIdentity.md

Please update the online version for these markdown files

cormacpayne · 2018-02-12T17:44:43Z

@maddieclayton would you mind taking a look at that changes Varun pushed, as well as helping him with the failing test?

MiYanni · 2018-02-12T18:54:41Z

@varunkch Can you also squash the commits? Thanks.

maddieclayton · 2018-02-12T21:54:45Z

@varunkch Once you have fixed the test (as outlined by Cormac over email) and squashed the commits, please assign this back to me.

maddieclayton

Once the online version is updated, this will be good to go. Ping me once this is done, and I will start a sign run.

maddieclayton · 2018-02-27T18:31:38Z

...nagedServiceIdentity/Commands.ManagedServiceIdentity/help/Get-AzureRmUserAssignedIdentity.md

+---
+external help file: Microsoft.Azure.Commands.ManagedServiceIdentity.dll-Help.xml
+Module Name: AzureRM.ManagedServiceIdentity
+online version: 1.0.0


All you need to do is follow the format, and it will create the proper link:

online version: https://docs.microsoft.com/en-us/powershell/module/azurerm.managedserviceidentity/get-azurermuserassignedidentity online version: https://docs.microsoft.com/en-us/powershell/module/azurerm.managedserviceidentity/new-azurermuserassignedidentity online version: https://docs.microsoft.com/en-us/powershell/module/azurerm.managedserviceidentity/remove-azurermuserassignedidentity

maddieclayton · 2018-02-27T18:44:56Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/515/

This reverts commit d2068b7.

maddieclayton · 2018-02-27T22:03:39Z

sign job here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/204/

maddieclayton · 2018-03-01T01:14:28Z

...anagedServiceIdentity/Commands.ManagedServiceIdentity/Commands.ManagedServiceIdentity.csproj

+      <Project>{3436a126-edc9-4060-8952-9a1be34cdd95}</Project>
+      <Name>Commands.ScenarioTests.ResourceManager.Common</Name>
+    </ProjectReference>
+    <ProjectReference Include="..\..\Profile\Commands.Profile\Commands.Profile.csproj">


You will need to remove both Profile and Resources in order to pass the sign job. Additionally, it seems like a lot of these project references are unnecessary (scenariotests for instance should only be in the test csproj). Please go through and remove the references that are not being used in your project.

Well I had added these references when you mentioned that I need to add all the common references from this file line 15-35 for sign job to pass:
https://github.com/Azure/azure-powershell/blob/preview/src/ResourceManager/Resources/Resources.sln#L15

I though we need to add all :)

Removed Resources, Profile and ScenarioTests, RBAC, Storage, Network since we dont need them

…nto msi_powershell2

This reverts commit a8d0e88.

maddieclayton · 2018-03-02T22:23:13Z

src/ResourceManager/ManagedServiceIdentity/ManagedServiceIdentity.sln

 VisualStudioVersion = 14.0.25420.1
 MinimumVisualStudioVersion = 10.0.40219.1
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Commands.ManagedServiceIdentity", "Commands.ManagedServiceIdentity\Commands.ManagedServiceIdentity.csproj", "{228EB071-FA04-43B3-95F9-7D76DBF0B850}"
+	ProjectSection(ProjectDependencies) = postProject


This doesn't look correct - please fix this so it follows the format below. Did you add the new project on visual studio?

varunkch · 2018-03-05T21:21:09Z

Sign job: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/234/

azuresdkci added the needs-review label Jan 12, 2018

azuresdkci assigned cormacpayne Jan 12, 2018

cormacpayne assigned markcowl and unassigned cormacpayne Jan 16, 2018

markcowl added Do Not Merge 🚫 and removed needs-review labels Jan 17, 2018

markcowl removed their assignment Jan 24, 2018

maddieclayton self-assigned this Jan 24, 2018

maddieclayton added the Preview label Jan 24, 2018

markcowl removed the Do Not Merge 🚫 label Jan 24, 2018

maddieclayton added the needs-review label Jan 24, 2018

cormacpayne added needs-revision and removed needs-review labels Jan 30, 2018

cormacpayne assigned varunkch and markcowl and unassigned maddieclayton and varunkch Jan 30, 2018

maddieclayton unassigned markcowl Feb 2, 2018

maddieclayton suggested changes Feb 6, 2018

View reviewed changes

cormacpayne added needs-review and removed needs-revision labels Feb 12, 2018

cormacpayne assigned maddieclayton and unassigned varunkch Feb 12, 2018

maddieclayton added the needs-revision label Feb 12, 2018

Addressing review comments

31eded9

maddieclayton reviewed Feb 27, 2018

View reviewed changes

Adding online version in help files

b8d9051

maddieclayton added Waiting for On Demand and removed needs-revision labels Feb 27, 2018

Varun Kumar added 4 commits February 27, 2018 11:34

Adding common projects to .sln file

d2068b7

Revert "Adding common projects to .sln file"

fc72908

This reverts commit d2068b7.

Adding common projects as references

d675ad7

Adding missing Nuget packages

e68bdaa

maddieclayton suggested changes Mar 1, 2018

View reviewed changes

maddieclayton added needs-revision and removed Waiting for On Demand labels Mar 1, 2018

Varun Kumar added 6 commits March 1, 2018 08:40

Removing unused references

ff11b19

Adding commands.resources to csproj

a8d0e88

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

c15a6fc

…nto msi_powershell2

Revert "Adding commands.resources to csproj"

a450dfe

This reverts commit a8d0e88.

Adding Commands.Resources to sln file

cb98905

Fixing Create_Mappings.json

547d138

maddieclayton suggested changes Mar 2, 2018

View reviewed changes

Varun Kumar added 2 commits March 5, 2018 08:24

Removing Commands.Resources from sln file

79fc393

Shortening test file names

823b094

maddieclayton approved these changes Mar 6, 2018

View reviewed changes

maddieclayton merged commit 4491ebc into Azure:preview Mar 6, 2018

maddieclayton added Waiting for On Demand and removed Preview needs-revision labels Mar 6, 2018

maddieclayton unassigned varunkch Mar 6, 2018

Adding new Powershell module for ManagedServiceIdentity #5270

Adding new Powershell module for ManagedServiceIdentity #5270

Uh oh!

Conversation

varunkch commented Jan 12, 2018 • edited by azuresdkci Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Adding new Powershell module for MSI (Managed Service Identity).

Uh oh!

varunkch commented Jan 12, 2018

Uh oh!

cormacpayne commented Jan 30, 2018

Uh oh!

maddieclayton 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!

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!

cormacpayne commented Feb 12, 2018

Uh oh!

MiYanni commented Feb 12, 2018

Uh oh!

maddieclayton commented Feb 12, 2018

Uh oh!

maddieclayton left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maddieclayton commented Feb 27, 2018

Uh oh!

maddieclayton commented Feb 27, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

varunkch Mar 1, 2018 • 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!

varunkch commented Mar 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

varunkch commented Jan 12, 2018 •

edited by azuresdkci

Loading

varunkch Mar 1, 2018 •

edited

Loading