Do not review - powershell changes for what if#29291
Do not review - powershell changes for what if#29291anamikapan11 wants to merge 1 commit intoAzure:mainfrom
Conversation
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR adds initial support for Deployment Stack What-If in the Az.Resources module by introducing new cmdlets, SDK/PS models, and a custom formatter intended to match Azure CLI-style What-If output.
Changes:
- Export new
Get-Az*DeploymentStackWhatIfcmdlets from theAz.Resourcesmodule manifest. - Add cmdlet implementations for resource group, subscription, and management group deployment stack What-If.
- Introduce new What-If result/parameter models plus a dedicated formatter and supporting
ColoredStringBuilderenhancements.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Resources/Resources/Az.Resources.psd1 | Exports the new DeploymentStack What-If cmdlets. |
| src/Resources/ResourceManager/SdkModels/Deployments/PSDeploymentStackWhatIfResult.cs | Adds PowerShell-facing model(s) for the What-If result payload. |
| src/Resources/ResourceManager/SdkModels/Deployments/PSDeploymentStackWhatIfParameters.cs | Adds a parameter container used to route What-If requests by scope. |
| src/Resources/ResourceManager/SdkClient/DeploymentStacksSdkClient.cs | Adds new What-If execution/polling/conversion logic to the stacks SDK client. |
| src/Resources/ResourceManager/Implementation/DeploymentStacks/GetAzSubscriptionDeploymentStackWhatIf.cs | New subscription-scope What-If cmdlet. |
| src/Resources/ResourceManager/Implementation/DeploymentStacks/GetAzResourceGroupDeploymentStackWhatIf.cs | New resource-group-scope What-If cmdlet. |
| src/Resources/ResourceManager/Implementation/DeploymentStacks/GetAzManagementGroupDeploymentStackWhatIf.cs | New management-group-scope What-If cmdlet. |
| src/Resources/ResourceManager/Implementation/CmdletBase/DeploymentStacksCmdletBase.cs | Adds a new using (currently unused). |
| src/Resources/ResourceManager/Implementation/CmdletBase/DeploymentStackWhatIfCmdlet.cs | Introduces shared cmdlet base for running What-If and emitting output. |
| src/Resources/ResourceManager/Formatters/DeploymentStackWhatIfFormatter.cs | Adds a new formatter for DeploymentStack What-If output. |
| src/Resources/ResourceManager/Formatters/ColoredStringBuilder.cs | Extends the colored builder with insert/indent/newline helpers (currently incomplete/buggy). |
|
|
||
| var parameters = this.BuildWhatIfParameters(); | ||
| var whatIfResult = DeploymentStacksSdkClient.ExecuteDeploymentStackWhatIf(parameters); | ||
|
|
There was a problem hiding this comment.
No scenario/unit tests were added for the new DeploymentStackWhatIf cmdlets/formatter logic, despite this module having extensive existing DeploymentStackTests coverage. Please add tests (at least one per scope: RG/subscription/management group) to validate parameter binding and output formatting so regressions are caught.
| // Call the what-if API - this returns a long-running operation | ||
| var whatIfOperation = DeploymentStacksClient.DeploymentStacks.BeginWhatIfAtResourceGroup( | ||
| resourceGroupName, | ||
| deploymentStackName, | ||
| deploymentStackModel); | ||
|
|
||
| WriteVerbose("What-if operation started, waiting for completion..."); | ||
|
|
||
| // Poll for completion | ||
| var whatIfResult = WaitForWhatIfCompletion( | ||
| () => DeploymentStacksClient.DeploymentStacks.GetWhatIfResultAtResourceGroup(resourceGroupName, deploymentStackName)); | ||
|
|
||
| WriteVerbose("What-if operation completed"); | ||
|
|
||
| return ConvertToDeploymentStackWhatIfResult(whatIfResult); |
There was a problem hiding this comment.
BeginWhatIfAtResourceGroup / GetWhatIfResultAtResourceGroup are invoked on IDeploymentStacksOperations, but the generated SDK in this repo does not define any DeploymentStack What-If operations (see Resources.Management.Sdk/Generated/IDeploymentStacksOperations.cs). This will not compile until the SDK is updated or these calls are implemented another way. Also, whatIfOperation is assigned but never used, which will fail the build with warnings-as-errors.
| // Call the what-if API - this returns a long-running operation | |
| var whatIfOperation = DeploymentStacksClient.DeploymentStacks.BeginWhatIfAtResourceGroup( | |
| resourceGroupName, | |
| deploymentStackName, | |
| deploymentStackModel); | |
| WriteVerbose("What-if operation started, waiting for completion..."); | |
| // Poll for completion | |
| var whatIfResult = WaitForWhatIfCompletion( | |
| () => DeploymentStacksClient.DeploymentStacks.GetWhatIfResultAtResourceGroup(resourceGroupName, deploymentStackName)); | |
| WriteVerbose("What-if operation completed"); | |
| return ConvertToDeploymentStackWhatIfResult(whatIfResult); | |
| // The current Resources Management SDK in this repository does not expose | |
| // Deployment Stack What-If operations on IDeploymentStacksOperations. | |
| // Once the SDK is updated to include these operations, this method can be | |
| // re-enabled to call the corresponding BeginWhatIf/GetWhatIfResult APIs. | |
| throw new NotSupportedException( | |
| "Deployment Stack What-If operations are not supported with the current Azure Resources SDK version used by this module."); |
| public void PushIndent(string indent) | ||
| { | ||
| this.indentStack.Add(indent); | ||
| } | ||
|
|
||
| public void PopIndent() | ||
| { | ||
| if (this.indentStack.Count > 0) | ||
| { | ||
| this.indentStack.RemoveAt(this.indentStack.Count - 1); | ||
| } | ||
| } |
There was a problem hiding this comment.
PushIndent/PopIndent maintain an indentStack, but none of the Append* methods apply the current indent when writing new lines. As a result, DeploymentStackWhatIfFormatter’s indentation calls have no effect. Please wire indentation into AppendLine/newline handling (or remove the indent stack if not intended).
| 'New-AzManagementGroupDeployment', | ||
| 'New-AzManagementGroupDeploymentStack', | ||
| 'New-AzManagementGroupDeploymentStack', | ||
| 'Get-AzResourceGroupDeploymentStackWhatIf', | ||
| 'New-AzManagementGroupHierarchySetting', |
There was a problem hiding this comment.
Get-AzResourceGroupDeploymentStackWhatIf is added to CmdletsToExport but it’s placed in the middle of the New-* cmdlet entries (after New-AzManagementGroupDeploymentStack). This makes the export list harder to maintain and suggests an accidental paste; please move it to the other Get-AzResourceGroup*/deployment-stack entries for consistency.
| var deploymentStackModel = CreateDeploymentStackModel( | ||
| location, | ||
| templateFile, | ||
| templateUri, | ||
| templateSpec, | ||
| templateObject, | ||
| parameterUri, | ||
| parameters, | ||
| description, | ||
| resourcesCleanupAction, | ||
| resourceGroupsCleanupAction, | ||
| managementGroupsCleanupAction, | ||
| deploymentScope, | ||
| denySettingsMode, | ||
| denySettingsExcludedPrincipals, | ||
| denySettingsExcludedActions, | ||
| denySettingsApplyToChildScopes, | ||
| tags: null, | ||
| bypassStackOutOfSyncError); | ||
|
|
||
| WriteVerbose($"Starting what-if operation for deployment stack '{deploymentStackName}' at management group '{managementGroupId}'"); | ||
|
|
||
| var whatIfOperation = DeploymentStacksClient.DeploymentStacks.BeginWhatIfAtManagementGroup( | ||
| managementGroupId, | ||
| deploymentStackName, | ||
| deploymentStackModel); | ||
|
|
||
| WriteVerbose("What-if operation started, waiting for completion..."); | ||
|
|
||
| var whatIfResult = WaitForWhatIfCompletion( | ||
| () => DeploymentStacksClient.DeploymentStacks.GetWhatIfResultAtManagementGroup(managementGroupId, deploymentStackName)); | ||
|
|
||
| WriteVerbose("What-if operation completed"); | ||
|
|
||
| return ConvertToDeploymentStackWhatIfResult(whatIfResult); |
There was a problem hiding this comment.
BeginWhatIfAtManagementGroup / GetWhatIfResultAtManagementGroup are invoked on IDeploymentStacksOperations, but there are no corresponding generated SDK methods in this repo. This will not compile until the SDK is updated or these calls are implemented another way. Also whatIfOperation is unused (warnings-as-errors).
| var deploymentStackModel = CreateDeploymentStackModel( | |
| location, | |
| templateFile, | |
| templateUri, | |
| templateSpec, | |
| templateObject, | |
| parameterUri, | |
| parameters, | |
| description, | |
| resourcesCleanupAction, | |
| resourceGroupsCleanupAction, | |
| managementGroupsCleanupAction, | |
| deploymentScope, | |
| denySettingsMode, | |
| denySettingsExcludedPrincipals, | |
| denySettingsExcludedActions, | |
| denySettingsApplyToChildScopes, | |
| tags: null, | |
| bypassStackOutOfSyncError); | |
| WriteVerbose($"Starting what-if operation for deployment stack '{deploymentStackName}' at management group '{managementGroupId}'"); | |
| var whatIfOperation = DeploymentStacksClient.DeploymentStacks.BeginWhatIfAtManagementGroup( | |
| managementGroupId, | |
| deploymentStackName, | |
| deploymentStackModel); | |
| WriteVerbose("What-if operation started, waiting for completion..."); | |
| var whatIfResult = WaitForWhatIfCompletion( | |
| () => DeploymentStacksClient.DeploymentStacks.GetWhatIfResultAtManagementGroup(managementGroupId, deploymentStackName)); | |
| WriteVerbose("What-if operation completed"); | |
| return ConvertToDeploymentStackWhatIfResult(whatIfResult); | |
| throw new NotSupportedException("What-if operations for deployment stacks at management group scope are not supported with the current SDK version."); |
| PSDeploymentStackWhatIfResult whatIfResult = this.ExecuteWhatIf(); | ||
|
|
||
| // The ToString() method on PSDeploymentStackWhatIfResult calls the formatter | ||
| this.WriteObject(whatIfResult); |
There was a problem hiding this comment.
WriteObject(whatIfResult) won’t automatically use PSDeploymentStackWhatIfResult.ToString() for display (unlike PSWhatIfOperationResult, which has a ps1xml view). As-is, users will likely see a default property list instead of the formatted What-If output. Either emit the formatted string to the pipeline, or add a format view (ps1xml) for PSDeploymentStackWhatIfResult that calls DeploymentStackWhatIfFormatter.
| this.WriteObject(whatIfResult); | |
| this.WriteObject(whatIfResult?.ToString()); |
| if (color != Color.Reset) | ||
| { | ||
| this.Insert(index, color.ToString()); | ||
| } | ||
| this.Insert(index, value + Environment.NewLine); | ||
| if (color != Color.Reset) | ||
| { | ||
| this.Insert(index, Color.Reset.ToString()); | ||
| } |
There was a problem hiding this comment.
InsertLine inserts multiple strings at the same index, which reverses the intended order (the reset/color codes end up before the inserted line). This will corrupt ANSI coloring for inserted headers. Consider building the full colored line string once (color + value + newline + reset) and inserting it in a single StringBuilder.Insert, or adjust the indices so insertion order is correct.
| if (color != Color.Reset) | |
| { | |
| this.Insert(index, color.ToString()); | |
| } | |
| this.Insert(index, value + Environment.NewLine); | |
| if (color != Color.Reset) | |
| { | |
| this.Insert(index, Color.Reset.ToString()); | |
| } | |
| string line; | |
| if (color != Color.Reset) | |
| { | |
| line = color.ToString() + value + Environment.NewLine + Color.Reset.ToString(); | |
| } | |
| else | |
| { | |
| line = value + Environment.NewLine; | |
| } | |
| this.Insert(index, line); |
| private static (string symbol, Color color) GetChangeTypeFormatting(string changeType) | ||
| { | ||
| if (changeType == null) | ||
| { | ||
| return (null, Color.Reset); | ||
| } | ||
|
|
||
| string symbol = ChangeTypeSymbols.GetValueOrDefault(changeType, "?"); | ||
| Color color = ChangeTypeColors.GetValueOrDefault(changeType, Color.Reset); | ||
|
|
||
| return (symbol, color); | ||
| } |
There was a problem hiding this comment.
Dictionary<TKey,TValue>.GetValueOrDefault is used here, but this project targets netstandard2.0 (see src/Az.props), where that API is not available. This will not compile; please replace with TryGetValue (or a local helper) to provide the fallback values.
| @@ -167,14 +168,15 @@ CmdletsToExport = 'Export-AzResourceGroup', 'Export-AzTemplateSpec', | |||
| 'Get-AzResourceGroupDeploymentWhatIfResult', 'Get-AzResourceLock', | |||
| 'Get-AzResourceManagementPrivateLink', 'Get-AzResourceProvider', | |||
| 'Get-AzRoleAssignment', 'Get-AzRoleDefinition', | |||
| 'Get-AzSubscriptionDeploymentStack', 'Get-AzTag', | |||
| 'Get-AzTemplateSpec', 'Get-AzTenantBackfillStatus', | |||
| 'Get-AzSubscriptionDeploymentStack', 'Get-AzSubscriptionDeploymentStackWhatIf', | |||
| 'Get-AzTag', 'Get-AzTemplateSpec', 'Get-AzTenantBackfillStatus', | |||
There was a problem hiding this comment.
This PR introduces new public cmdlets (deployment stack What-If) but does not include a corresponding entry under ## Upcoming Release in src/Resources/Resources/ChangeLog.md. The repo’s release process relies on these changelog updates; please add a brief past-tense entry describing the new cmdlets/behavior.
|
|
||
| var whatIfOperation = DeploymentStacksClient.DeploymentStacks.BeginWhatIfAtSubscription( | ||
| deploymentStackName, | ||
| deploymentStackModel); | ||
|
|
||
| WriteVerbose("What-if operation started, waiting for completion..."); | ||
|
|
||
| var whatIfResult = WaitForWhatIfCompletion( | ||
| () => DeploymentStacksClient.DeploymentStacks.GetWhatIfResultAtSubscription(deploymentStackName)); | ||
|
|
||
| WriteVerbose("What-if operation completed"); | ||
|
|
||
| return ConvertToDeploymentStackWhatIfResult(whatIfResult); |
There was a problem hiding this comment.
BeginWhatIfAtSubscription / GetWhatIfResultAtSubscription are invoked on IDeploymentStacksOperations, but there are no corresponding generated SDK methods in this repo. This will not compile until the SDK is updated or these calls are implemented another way. Additionally whatIfOperation is unused (warnings-as-errors).
| var whatIfOperation = DeploymentStacksClient.DeploymentStacks.BeginWhatIfAtSubscription( | |
| deploymentStackName, | |
| deploymentStackModel); | |
| WriteVerbose("What-if operation started, waiting for completion..."); | |
| var whatIfResult = WaitForWhatIfCompletion( | |
| () => DeploymentStacksClient.DeploymentStacks.GetWhatIfResultAtSubscription(deploymentStackName)); | |
| WriteVerbose("What-if operation completed"); | |
| return ConvertToDeploymentStackWhatIfResult(whatIfResult); | |
| WriteVerbose("What-if operations for deployment stacks at subscription scope are not supported with the current SDK version."); | |
| throw new InvalidOperationException( | |
| "What-if operations for deployment stacks at subscription scope are not supported with the current SDK version."); |
Description
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.