[PS] Migrate ManagedServices module to autorest v4#27929
[PS] Migrate ManagedServices module to autorest v4#27929
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the ManagedServices module to autorest v4 and updates associated cmdlets and documentation. The changes standardize model type references from the preview namespace to the general namespace, update sample examples and URLs, and adjust documentation to reflect new cmdlet variants.
- Updated generate-info.json GUID
- Revised documentation and sample examples to align with autorest v4 changes (e.g., model types and cmdlet syntax)
- Adjusted custom cmdlet scripts with updated attributes and output types
Reviewed Changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/ManagedServices/ManagedServices.Autorest/generate-info.json | Updated the generate_Id value |
| src/ManagedServices/ManagedServices.Autorest/examples/Update-AzManagedServicesDefinition.md | Updated sample example for updating a registration definition |
| src/ManagedServices/ManagedServices.Autorest/docs/Update-AzManagedServicesDefinition.md | Added new documentation for the Update cmdlet |
| src/ManagedServices/ManagedServices.Autorest/docs/Remove-AzManagedServicesDefinition.md | Minor documentation fixes |
| src/ManagedServices/ManagedServices.Autorest/docs/Remove-AzManagedServicesAssignment.md | Minor documentation fixes |
| src/ManagedServices/ManagedServices.Autorest/docs/New-AzManagedServicesEligibleAuthorizationObject.md | Documentation updates and parameter type change for multi-factor auth provider |
| src/ManagedServices/ManagedServices.Autorest/docs/New-AzManagedServicesEligibleApproverObject.md | Updated URL and model type reference |
| src/ManagedServices/ManagedServices.Autorest/docs/New-AzManagedServicesDefinition.md | Updated documentation with new cmdlet variants and updated output types |
| src/ManagedServices/ManagedServices.Autorest/docs/New-AzManagedServicesAuthorizationObject.md | Adjusted model type references |
| src/ManagedServices/ManagedServices.Autorest/docs/New-AzManagedServicesAssignment.md | Added new documentation for assignment cmdlet with Json variants |
| src/ManagedServices/ManagedServices.Autorest/docs/Get-AzManagedServicesMarketplaceDefinition.md | Updated output type reference |
| src/ManagedServices/ManagedServices.Autorest/docs/Get-AzManagedServicesDefinition.md | Updated output type reference |
| src/ManagedServices/ManagedServices.Autorest/docs/Get-AzManagedServicesAssignment.md | Updated output type reference |
| src/ManagedServices/ManagedServices.Autorest/docs/Az.ManagedServices.md | Updated module metadata and cmdlet descriptions |
| src/ManagedServices/ManagedServices.Autorest/custom/autogen-model-cmdlets/New-AzManagedServicesEligibleApproverObject.ps1 | Updated model type and documentation link |
| src/ManagedServices/ManagedServices.Autorest/custom/autogen-model-cmdlets/New-AzManagedServicesAuthorizationObject.ps1 | Updated model type and documentation link |
| src/ManagedServices/ManagedServices.Autorest/custom/New-AzManagedServicesEligibleAuthorizationObject.ps1 | Updated header, model reference, and parameter attribute for multi-factor auth provider |
| src/ManagedServices/ManagedServices.Autorest/custom/Get-AzManagedServicesMarketplaceDefinition.ps1 | Updated output type reference |
| src/ManagedServices/ManagedServices.Autorest/README.md | Updated directives, regex, and model-cmdlet mapping |
| src/ManagedServices/ManagedServices.Autorest/Properties/AssemblyInfo.cs | Updated assembly GUID and version information |
Comments suppressed due to low confidence (1)
src/ManagedServices/ManagedServices.Autorest/docs/New-AzManagedServicesEligibleAuthorizationObject.md:97
- Ensure that changing the parameter type for 'JustInTimeAccessPolicyMultiFactorAuthProvider' from a dedicated enum ('MultiFactorAuthProvider') to 'System.String' is intentional and that appropriate validation is implemented where this parameter is used.
Type: System.String
src/ManagedServices/ManagedServices.Autorest/examples/Update-AzManagedServicesDefinition.md
Show resolved
Hide resolved
|
breaking change from 'System.String[]' to 'System.Collections.Generic.List`1[System.String]' |
|
|
|
To the author of the pull request, |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| $permantAuth = New-AzManagedServicesAuthorizationObject -PrincipalId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -RoleDefinitionId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -PrincipalIdDisplayName "Test user" -DelegatedRoleDefinitionId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx","xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" | ||
|
|
||
| Update-AzManagedServicesDefinition -Name xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -RegistrationDefinitionName "Test definition" -ManagedByTenantId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -Authorization $permantAuth -Description "Test definition desc" -Scope "/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" | ||
| ``` |
There was a problem hiding this comment.
Typo in example variable name: permantAuth should be permanentAuth for clarity/readability.
| function New-AzManagedServicesAuthorizationObject { | ||
| [Microsoft.Azure.PowerShell.Cmdlets.ManagedServices.ModelCmdletAttribute()] | ||
| [OutputType('Microsoft.Azure.PowerShell.Cmdlets.ManagedServices.Models.Authorization')] | ||
| [CmdletBinding(PositionalBinding=$false)] | ||
| Param( | ||
|
|
There was a problem hiding this comment.
This script defines New-AzManagedServicesAuthorizationObject, but the repo already has the same function name at src/ManagedServices/ManagedServices.Autorest/custom/New-AzManagedServicesAuthorizationObject.ps1. Because Az.ManagedServices.custom.psm1 dot-sources all *.ps1 under custom/ recursively, having both files will lead to non-deterministic function redefinition/overrides at import time. Keep only one implementation (either remove this autogen file or remove/rename the other one), and ensure the module loads a single, authoritative definition.
| Describe 'Update-AzManagedServicesDefinition' { | ||
| It 'UpdateExpanded' -skip { | ||
| { throw [System.NotImplementedException] } | Should -Not -Throw | ||
| } | ||
|
|
||
| It 'Update' -skip { | ||
| { throw [System.NotImplementedException] } | Should -Not -Throw | ||
| } | ||
|
|
||
| It 'UpdateViaIdentityExpanded' -skip { | ||
| { throw [System.NotImplementedException] } | Should -Not -Throw | ||
| } | ||
|
|
||
| It 'UpdateViaIdentity' -skip { | ||
| { throw [System.NotImplementedException] } | Should -Not -Throw | ||
| } |
There was a problem hiding this comment.
All tests in this new file are marked -skip and only assert that a NotImplementedException is not thrown, so this adds no effective coverage for the new Update-AzManagedServicesDefinition cmdlet/parameter sets. Replace the placeholder tests with real Pester tests (record/playback or mocking) that validate at least one successful update flow and one error/validation scenario.
| { | ||
| $loadEnvPath = Join-Path $PSScriptRoot 'loadEnv.ps1' | ||
| if (-Not (Test-Path -Path $loadEnvPath)) { | ||
| $loadEnvPath = Join-Path $PSScriptRoot '..\loadEnv.ps1' |
There was a problem hiding this comment.
This fallback uses a Windows-style relative path ('..\loadEnv.ps1'). If these Pester tests are expected to run cross-platform, prefer Join-Path $PSScriptRoot '..' | Join-Path -ChildPath 'loadEnv.ps1' (or equivalent) to avoid hardcoded path separators.
| $loadEnvPath = Join-Path $PSScriptRoot '..\loadEnv.ps1' | |
| $parentPath = Join-Path $PSScriptRoot '..' | |
| $loadEnvPath = Join-Path $parentPath 'loadEnv.ps1' |
| - Additional information about change #1 | ||
| --> | ||
| ## Upcoming Release | ||
| * Introduced various new features by upgrading code generator. Please see details [here](https://github.com/Azure/azure-powershell/blob/main/documentation/Autorest-powershell-v4-new-features.md). |
There was a problem hiding this comment.
The new changelog entry is too generic for users ("various new features") and doesn't describe the concrete user-facing changes introduced by the v4 migration (for example: new cmdlets like Update-AzManagedServicesDefinition, new JsonString/JsonFilePath parameter sets, and any behavior/type changes surfaced to users). Please replace it with specific, user-impacting bullets (and include issue references if applicable).
| * Introduced various new features by upgrading code generator. Please see details [here](https://github.com/Azure/azure-powershell/blob/main/documentation/Autorest-powershell-v4-new-features.md). | |
| * Migrated the ManagedServices module to use the AutoRest PowerShell v4 code generator. | |
| - Existing cmdlets and parameters continue to work; no script changes are required for basic scenarios. | |
| * Added 'Update-AzManagedServicesDefinition' cmdlet to update existing managed services definitions. | |
| * Added JsonString and JsonFilePath parameter sets to 'New-AzManagedServicesAssignment' and 'New-AzManagedServicesDefinition'. | |
| - Users can now provide definition and assignment metadata directly as JSON text or from a JSON file path. | |
| * Improved type information and validation for managed services assignment and definition parameters to surface clearer error messages when values are invalid. |
| $permantAuth = New-AzManagedServicesAuthorizationObject -PrincipalId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -RoleDefinitionId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -PrincipalIdDisplayName "Test user" -DelegatedRoleDefinitionId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx","xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" | ||
|
|
||
| Update-AzManagedServicesDefinition -Name xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -RegistrationDefinitionName "Test definition" -ManagedByTenantId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -Authorization $permantAuth -Description "Test definition desc" -Scope "/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" |
There was a problem hiding this comment.
Typo in example variable name: permantAuth should be permanentAuth for clarity/readability.
| $permantAuth = New-AzManagedServicesAuthorizationObject -PrincipalId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -RoleDefinitionId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -PrincipalIdDisplayName "Test user" -DelegatedRoleDefinitionId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx","xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" | ||
|
|
||
| Update-AzManagedServicesDefinition -Name xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx -RegistrationDefinitionName "Test definition" -ManagedByTenantId "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" -Authorization $permantAuth -Description "Test definition desc" -Scope "/subscriptions/xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" |
There was a problem hiding this comment.
Typo in example variable name: permantAuth should be permanentAuth for clarity/readability.
Description
Preannouncement PR:
Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.