-Confirm parameter value not respected by Remove-AzResourceGroup

[bergmeister]

10471 Description

When passing in -Confirm:$false it should override confirmation prompts so that one does not need to the use the -Force parameter which is very overloaded and dangerous. However, it still prompts and I have to use the -Force parameter

Steps to reproduce

Remove-AzResourceGroup -ResourceGroupName $ResourceGroupName -Confirm:$false

Example in CloudShell:

Environment data

CloudShell

Name                           Value
----                           -----
PSVersion                      6.2.3
PSEdition                      Core
GitCommitId                    6.2.3
OS                             Linux 4.15.0-1063-azure #68-Ubuntu SMP Fri Nov 8 09:30:20 UTC 2019
Platform                       Unix
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Module versions

    Directory: /home/christoph/.local/share/powershell/Modules

ModuleType Version    Name                                PSEdition ExportedCommands
---------- -------    ----                                --------- ----------------
Script     1.1903.0.… DockerCompletion                    Desk
Script     0.1        MavenAutoCompletion                 Desk
Script     0.0.1      npm-completion                      Desk
Script     4.9.0      Pester                              Desk      {Describe, Context, It, Should…}
Script     0.3        posh-cli                            Core,Desk Install-TabCompletion
Script     0.0.2      posh-dcos                           Desk
Script     1.2.3      posh-dotnet                         Desk
Script     0.7.3      posh-git                            Desk      {Invoke-NullCoalescing, Add-PoshGitToProfile, Get-PromptPath, Write-GitStatus…}
Script     1.18.2     PSScriptAnalyzer                    Desk      {Get-ScriptAnalyzerRule, Invoke-ScriptAnalyzer, Invoke-Formatter}
Script     1.2        TabExpansionPlusPlus                Desk      {Get-CommandWithParameter, New-CompletionResult, Get-CompletionWithExtension, Get-ArgumentCompleter…}

    Directory: /usr/local/share/powershell/Modules

ModuleType Version    Name                                PSEdition ExportedCommands
---------- -------    ----                                --------- ----------------
Script     3.0.0      Az                                  Core,Desk
Script     1.6.4      Az.Accounts                         Core,Desk {Disable-AzDataCollection, Disable-AzContextAutosave, Enable-AzDataCollection, Enable-AzContextAutosave…}
Script     1.1.0      Az.Advisor                          Core,Desk {Get-AzAdvisorRecommendation, Enable-AzAdvisorRecommendation, Disable-AzAdvisorRecommendation, Get-AzAdvisorConfiguration…}
Script     1.0.2      Az.Aks                              Core,Desk {Get-AzAks, New-AzAks, Remove-AzAks, Import-AzAksCredential…}
Script     1.1.1      Az.AnalysisServices                 Core,Desk {Resume-AzAnalysisServicesServer, Suspend-AzAnalysisServicesServer, Get-AzAnalysisServicesServer, Remove-AzAnalysisServicesServer…}
Script     1.3.2      Az.ApiManagement                    Core,Desk {Add-AzApiManagementApiToProduct, Add-AzApiManagementProductToGroup, Add-AzApiManagementRegion, Add-AzApiManagementUserToGroup…}
Script     1.0.2      Az.ApplicationInsights              Core,Desk {Get-AzApplicationInsights, New-AzApplicationInsights, Remove-AzApplicationInsights, Set-AzApplicationInsightsPricingPlan…}
Script     1.3.4      Az.Automation                       Core,Desk {Get-AzAutomationHybridWorkerGroup, Remove-AzAutomationHybridWorkerGroup, Get-AzAutomationJobOutputRecord, Import-AzAutomationDscNodeConfiguration…}
Script     2.0.1      Az.Batch                            Core,Desk {Remove-AzBatchAccount, Get-AzBatchAccount, Get-AzBatchAccountKey, New-AzBatchAccount…}
Script     1.0.1      Az.Billing                          Core,Desk {Get-AzBillingInvoice, Get-AzBillingPeriod, Get-AzEnrollmentAccount, Get-AzConsumptionBudget…}
Script     1.4.0      Az.Cdn                              Core,Desk {Get-AzCdnProfile, Get-AzCdnProfileSsoUrl, New-AzCdnProfile, Remove-AzCdnProfile…}
Script     1.2.1      Az.CognitiveServices                Core,Desk {Get-AzCognitiveServicesAccount, Get-AzCognitiveServicesAccountKey, Get-AzCognitiveServicesAccountSku, Get-AzCognitiveServicesAccountType…}
Script     3.0.0      Az.Compute                          Core,Desk {Remove-AzAvailabilitySet, Get-AzAvailabilitySet, New-AzAvailabilitySet, Update-AzAvailabilitySet…}
Script     1.0.1      Az.ContainerInstance                Core,Desk {New-AzContainerGroup, Get-AzContainerGroup, Remove-AzContainerGroup, Get-AzContainerInstanceLog}
Script     1.1.0      Az.ContainerRegistry                Core,Desk {New-AzContainerRegistry, Get-AzContainerRegistry, Update-AzContainerRegistry, Remove-AzContainerRegistry…}
Script     1.4.1      Az.DataFactory                      Core,Desk {Set-AzDataFactoryV2, Update-AzDataFactoryV2, Get-AzDataFactoryV2, Remove-AzDataFactoryV2…}
Script     1.0.1      Az.DataLakeAnalytics                Core,Desk {Get-AzDataLakeAnalyticsDataSource, New-AzDataLakeAnalyticsCatalogCredential, Remove-AzDataLakeAnalyticsCatalogCredential, Set-AzDataLakeAnalyticsCatalogCredential…}
Script     1.2.4      Az.DataLakeStore                    Core,Desk {Get-AzDataLakeStoreTrustedIdProvider, Remove-AzDataLakeStoreTrustedIdProvider, Remove-AzDataLakeStoreFirewallRule, Set-AzDataLakeStoreTrustedIdProvider…}
Script     1.0.1      Az.DeploymentManager                Core,Desk {Get-AzDeploymentManagerArtifactSource, New-AzDeploymentManagerArtifactSource, Set-AzDeploymentManagerArtifactSource, Remove-AzDeploymentManagerArtifactSource…}
Script     1.0.0      Az.DevTestLabs                      Core,Desk {Get-AzDtlAllowedVMSizesPolicy, Get-AzDtlAutoShutdownPolicy, Get-AzDtlAutoStartPolicy, Get-AzDtlVMsPerLabPolicy…}
Script     1.1.1      Az.Dns                              Core,Desk {Get-AzDnsRecordSet, New-AzDnsRecordConfig, Remove-AzDnsRecordSet, Set-AzDnsRecordSet…}
Script     1.2.2      Az.EventGrid                        Core,Desk {New-AzEventGridTopic, Get-AzEventGridTopic, Set-AzEventGridTopic, New-AzEventGridTopicKey…}
Script     1.4.0      Az.EventHub                         Core,Desk {New-AzEventHubNamespace, Get-AzEventHubNamespace, Set-AzEventHubNamespace, Remove-AzEventHubNamespace…}
Script     1.1.2      Az.FrontDoor                        Core,Desk {New-AzFrontDoor, Get-AzFrontDoor, Set-AzFrontDoor, Remove-AzFrontDoor…}
Script     0.10.6     Az.GuestConfiguration               Core,Desk {Get-AzVMGuestPolicyStatus, Get-AzVMGuestPolicyStatusHistory}
Script     3.0.0      Az.HDInsight                        Core,Desk {Get-AzHDInsightJob, New-AzHDInsightSqoopJobDefinition, Wait-AzHDInsightJob, New-AzHDInsightStreamingMapReduceJobDefinition…}
Script     1.0.0      Az.HealthcareApis                   Core,Desk {New-AzHealthcareApisService, Remove-AzHealthcareApisService, Set-AzHealthcareApisService, Get-AzHealthcareApisService}
Script     2.0.0      Az.IotHub                           Core,Desk {Add-AzIotHubKey, Get-AzIotHubEventHubConsumerGroup, Get-AzIotHubConnectionString, Get-AzIotHubJob…}
Script     1.3.1      Az.KeyVault                         Core,Desk {Add-AzKeyVaultCertificate, Update-AzKeyVaultCertificate, Stop-AzKeyVaultCertificateOperation, Get-AzKeyVaultCertificateOperation…}
Script     1.3.1      Az.LogicApp                         Core,Desk {Get-AzIntegrationAccountAgreement, Get-AzIntegrationAccountAssembly, Get-AzIntegrationAccountBatchConfiguration, Get-AzIntegrationAccountCallbackUrl…}
Script     1.1.1      Az.MachineLearning                  Core,Desk {Move-AzMlCommitmentAssociation, Get-AzMlCommitmentAssociation, Get-AzMlCommitmentPlanUsageHistory, Remove-AzMlCommitmentPlan…}
Script     1.0.1      Az.ManagedServices                  Core,Desk {Get-AzManagedServicesAssignment, New-AzManagedServicesAssignment, Remove-AzManagedServicesAssignment, Get-AzManagedServicesDefinition…}
Script     1.0.1      Az.MarketplaceOrdering              Core,Desk {Get-AzMarketplaceTerms, Set-AzMarketplaceTerms}
Script     1.1.0      Az.Media                            Core,Desk {Sync-AzMediaServiceStorageKey, Set-AzMediaServiceKey, Get-AzMediaServiceKey, Get-AzMediaServiceNameAvailability…}
Script     1.4.0      Az.Monitor                          Core,Desk {Get-AzMetricDefinition, Get-AzMetric, Remove-AzLogProfile, Get-AzLogProfile…}
Script     2.0.0      Az.Network                          Core,Desk {Add-AzApplicationGatewayAuthenticationCertificate, Get-AzApplicationGatewayAuthenticationCertificate, New-AzApplicationGatewayAuthenticationCertificate, Remove-AzAppl…
Script     1.1.0      Az.NotificationHubs                 Core,Desk {Get-AzNotificationHub, Get-AzNotificationHubAuthorizationRule, Get-AzNotificationHubListKey, Get-AzNotificationHubPNSCredential…}
Script     1.3.3      Az.OperationalInsights              Core,Desk {New-AzOperationalInsightsAzureActivityLogDataSource, New-AzOperationalInsightsCustomLogDataSource, Disable-AzOperationalInsightsLinuxCustomLogCollection, Disable-AzOp…
Script     1.1.3      Az.PolicyInsights                   Core,Desk {Get-AzPolicyEvent, Get-AzPolicyState, Get-AzPolicyStateSummary, Get-AzPolicyRemediation…}
Script     1.1.0      Az.PowerBIEmbedded                  Core,Desk {Remove-AzPowerBIWorkspaceCollection, Get-AzPowerBIWorkspaceCollection, Get-AzPowerBIWorkspaceCollectionAccessKey, Get-AzPowerBIWorkspace…}
Script     1.0.0      Az.PrivateDns                       Core,Desk {Get-AzPrivateDnsZone, Remove-AzPrivateDnsZone, Set-AzPrivateDnsZone, New-AzPrivateDnsZone…}
Script     2.0.1      Az.RecoveryServices                 Core,Desk {Get-AzRecoveryServicesBackupProperty, Get-AzRecoveryServicesVault, Get-AzRecoveryServicesVaultSettingsFile, New-AzRecoveryServicesVault…}
Script     1.1.1      Az.RedisCache                       Core,Desk {Remove-AzRedisCachePatchSchedule, New-AzRedisCacheScheduleEntry, Get-AzRedisCachePatchSchedule, New-AzRedisCachePatchSchedule…}
Script     1.0.2      Az.Relay                            Core,Desk {New-AzRelayNamespace, Get-AzRelayNamespace, Set-AzRelayNamespace, Remove-AzRelayNamespace…}
Script     1.7.1      Az.Resources                        Core,Desk {Get-AzProviderOperation, Remove-AzRoleAssignment, Get-AzRoleAssignment, New-AzRoleAssignment…}
Script     1.4.0      Az.ServiceBus                       Core,Desk {New-AzServiceBusNamespace, Get-AzServiceBusNamespace, Set-AzServiceBusNamespace, Remove-AzServiceBusNamespace…}
Script     2.0.0      Az.ServiceFabric                    Core,Desk {Add-AzServiceFabricClientCertificate, Add-AzServiceFabricClusterCertificate, Add-AzServiceFabricNode, Add-AzServiceFabricNodeType…}
Script     1.1.0      Az.SignalR                          Core,Desk {New-AzSignalR, Get-AzSignalR, Get-AzSignalRKey, New-AzSignalRKey…}
Script     2.0.0      Az.Sql                              Core,Desk {Get-AzSqlDatabaseTransparentDataEncryption, Get-AzSqlDatabaseTransparentDataEncryptionActivity, Set-AzSqlDatabaseTransparentDataEncryption, Get-AzSqlDatabaseUpgradeHi…
Script     1.9.0      Az.Storage                          Core,Desk {Get-AzStorageAccount, Get-AzStorageAccountKey, New-AzStorageAccount, New-AzStorageAccountKey…}
Script     1.2.1      Az.StorageSync                      Core,Desk {Invoke-AzStorageSyncCompatibilityCheck, New-AzStorageSyncService, Get-AzStorageSyncService, Remove-AzStorageSyncService…}
Script     1.0.0      Az.StreamAnalytics                  Core,Desk {Get-AzStreamAnalyticsFunction, Get-AzStreamAnalyticsDefaultFunctionDefinition, New-AzStreamAnalyticsFunction, Remove-AzStreamAnalyticsFunction…}
Script     1.0.2      Az.TrafficManager                   Core,Desk {Add-AzTrafficManagerCustomHeaderToEndpoint, Remove-AzTrafficManagerCustomHeaderFromEndpoint, Add-AzTrafficManagerCustomHeaderToProfile, Remove-AzTrafficManagerCustomH…
Script     1.5.0      Az.Websites                         Core,Desk {Get-AzAppServicePlan, Set-AzAppServicePlan, New-AzAppServicePlan, Remove-AzAppServicePlan…}
Script     0.0.0.10   AzureAD.Standard.Preview            Desk      {Get-AzureADApplicationPasswordCredential, Remove-AzureADServiceAppRoleAssignment, Set-AzureADApplicationProxyConnectorGroup, Get-AzureADDeletedApplication…}
Script     0.9.3      AzurePSDrive                        Desk
Script     17.0.2985… EXOPSSessionConnector               Desk      Connect-EXOPSSession
Manifest   1.0.802    MicrosoftPowerBIMgmt                Desk
Binary     1.0.802    MicrosoftPowerBIMgmt.Admin          Desk      {Add-PowerBIEncryptionKey, Get-PowerBIEncryptionKey, Get-PowerBIWorkspaceEncryptionStatus, Switch-PowerBIEncryptionKey…}
Binary     1.0.802    MicrosoftPowerBIMgmt.Capacities     Desk      Get-PowerBICapacity
Binary     1.0.802    MicrosoftPowerBIMgmt.Data           Desk      {Add-PowerBIDataset, Set-PowerBITable, New-PowerBIDataset, New-PowerBITable…}
Binary     1.0.802    MicrosoftPowerBIMgmt.Profile        Desk      {Connect-PowerBIServiceAccount, Disconnect-PowerBIServiceAccount, Invoke-PowerBIRestMethod, Get-PowerBIAccessToken…}
Binary     1.0.802    MicrosoftPowerBIMgmt.Reports        Desk      {Get-PowerBIReport, New-PowerBIReport, Export-PowerBIReport, Get-PowerBIDashboard…}
Binary     1.0.802    MicrosoftPowerBIMgmt.Workspaces     Desk      {Get-PowerBIWorkspace, Get-PowerBIWorkspaceMigrationStatus, Add-PowerBIWorkspaceUser, Remove-PowerBIWorkspaceUser…}
Binary     1.0.2      MicrosoftTeams                      Desk      {Get-CsBatchPolicyAssignmentOperation, Invoke-CsBatchPolicyAssignment}
Script     0.9.3      PSCloudShellUtility                 Desk      {Enter-AzVM, Get-AzCommand, Invoke-AzVMCommand, Enable-AzVMPSRemoting…}
Binary     0.8.1      SHiPS                               Desk
Script     21.1.18206 SqlServer                           Desk      {Add-RoleMember, Add-SqlAvailabilityDatabase, Add-SqlAvailabilityGroupListenerStaticIp, Add-SqlAzureAuthenticationContext…}

    Directory: /opt/microsoft/powershell/6/Modules

ModuleType Version    Name                                PSEdition ExportedCommands
---------- -------    ----                                --------- ----------------
Manifest   1.2.3.0    Microsoft.PowerShell.Archive        Desk      {Compress-Archive, Expand-Archive}
Manifest   6.1.0.0    Microsoft.PowerShell.Host           Core      {Start-Transcript, Stop-Transcript}
Manifest   6.1.0.0    Microsoft.PowerShell.Management     Core      {Add-Content, Clear-Content, Clear-ItemProperty, Join-Path…}
Manifest   6.1.0.0    Microsoft.PowerShell.Security       Core      {Get-Credential, Get-ExecutionPolicy, Set-ExecutionPolicy, ConvertFrom-SecureString…}
Manifest   6.1.0.0    Microsoft.PowerShell.Utility        Core      {Export-Alias, Get-Alias, Import-Alias, New-Alias…}
Script     1.3.2      PackageManagement                   Desk      {Find-Package, Get-Package, Get-PackageProvider, Get-PackageSource…}
Script     2.1.3      PowerShellGet                       Desk      {Find-Command, Find-DSCResource, Find-Module, Find-RoleCapability…}
Script     0.0        PSDesiredStateConfiguration         Desk      {Update-DependsOn, Update-LocalConfigManager, Get-EncryptedPassword, Test-NodeManager…}
Script     2.0.0      PSReadLine                          Desk      {Get-PSReadLineKeyHandler, Set-PSReadLineKeyHandler, Remove-PSReadLineKeyHandler, Get-PSReadLineOption…}
Binary     1.1.2      ThreadJob                           Desk      Start-ThreadJob

[erich-wang]

@bergmeister , this cmdlet is implemented with both ShouldContinue and ShouldProcess, -Confirm:$false can only override ShouldProcess, you have to specify -Force to override ShouldContinue. Please let us know if any comment, thanks.

[vexx32]

@erich-wang why are you using ShouldContinue? In the vast majority of use cases ShouldProcess is more than sufficient. If you need to prompt by default, typically setting ConfirmImpact = "High" is the recommended route.

ShouldContinue and -Force are generally a bad pattern to follow, not least because the prompts generated by ShouldContinue are indistinguishable from ShouldProcess.

[erich-wang]

From @markcowl:

-Confirm is an automatic parameter completely handled by PowerShell. It impacts the display of ShouldProcess prompts without intervention from the cmdlet.

-Force is a parameter that is defined by convention for any cmdlet that also implements ShouldContinue prompts, and avoids the Should continue prompts.

It is entirely expected that settings to the -Confirm parameter have no impact whatsoever on the ShouldContinue.

This may be confusing to people who are not initimately familiar with PowerShell confirmation, but we need to respect the actual confirmation conventions of PowerShell.

You can find more about confirmation here:

https://docs.microsoft.com/en-us/powershell/scripting/developer/cmdlet/how-to-request-confirmations?view=powershell-6

or you can see a really detailed discussion of this in this gist, when we made cmdlet confirmation compliant with PowerShell recommendations across the board. This specific case is mentioned there: https://gist.github.com/markcowl/338e16fe5c8bbf195aff9f8af0db585d

[erich-wang]

@vexx32, we'll close this issue as by design because of it is recommended implementation by PowerShell team. Please let us know if any concern, or you could raise one discussion with powershell team at https://github.com/PowerShell/PowerShell/issues, thanks.

[vexx32]

@erich-wang Guidance in the listed article is from 2016. I've not seen a cmdlet introduced since well before that point that implements a -Force parameter.

Unlike -Confirm and -WhatIf, -Force is not an automatic parameter that is bundled with ShouldProcess. It's a kludge, at best, in my opinion. ShouldContinue() is not a necessary method, and -- again, in my opinion -- is best left as an implementation detail for ShouldProcess()'s own prompting use, as I understand it.

However, there are the occasional case where it may be considered "necessary" to always prompt. In my own opinion, that's kind of exactly what ConfirmImpact exists for -- set it to High and it will always prompt unless a user has deliberately altered their $ConfirmPreference variable to bypass it.

The PowerShell team may have their own guidance (/cc @SteveL-MSFT for additional perspective).

I'm rather surprised that such a method is being used, and even more surprised that it is supposedly recommended to essentially re-implement what is already being done with ShouldProcess(). The example from the article is:

protected override void ProcessRecord()
{
  if (ShouldProcess("ShouldProcess target"))
  {
    if (Force || ShouldContinue("", ""))
    {
      // Add code that performs the operation.
    }
  }
}

This is lacking information. @sdwheeler and @SteveL-MSFT: I think we should update that article. It only refers to use of ShouldContinue; use of ShouldProcess() on its own is generally sufficient for all but the most harmful operation.

@erich-wang from a more pragmatic standpoint, I don't think it helps your users to expect them to be intimately familiar with the mechanisms with which PowerShell handles prompting. If you're providing a -Confirm parameter by implementing ShouldProcess(), your cmdlet should be respecting the value of said parameter unless the operation being performed should always be prompted for regardless of user preferences.

In general, such prompts are reserved only for unduly / unexpectedly harmful operations. The common example is trying to delete a non-empty directory with Remove-Item and not supplying -Recurse; in this case, the invocation could be a mistake, the user may have wanted to target a file instead, and deleting an entire directory could be hazardous.

Is this similarly hazardous / unexpected behaviour? As I understand the purpose of the cmdlet, I don't think the mandatory / unavoidable prompt is necessary here.

[bergmeister]

Yes, I agree with @vexx32 that although this used to be a recommendation from the PowerShell team, the practical experience from various cmdlets (the most famous one being install-module) showed that the original recommendation has some downsides that only showed up over time as functionality of cmdlets grew and I think most people would agree that it's now more of an anti-pattern that should not be used any more in modern PowerShell. The main problem with the approach is not only that one cannot use the confirm parameter (which would be the intuitive UX) but moreover that the Force parameter grows slowly to a dangerous beast, where users confirm the action without knowing which warning messages are being ignored as the force parameter ignores all of them. This contradicts modern security where one should only turn off the warnings one by one for a better understanding and reduced vulnerability surface because there could be cases where force is used but important warnings are ignored that might not be desired to be ignored, therefore the user does not have much of a choice. Whilst it is nice to have a single switch to say 'just do it, I take responsibility', one also needs to be given the opportunity to only opt into ignoring some warnings, which brings us back to the usage of shouldprocess/shouldcontinue where the non-interactive scenario needs to work as well, therefore the best solution seems to be here to not use ShouldContinue but rather ShouldProcess. Since you cannot remove the force parameter without a breaking change, I suggest an additional parameter for some cmdlets where there is more than one ShouldProcess command to allow the user to override a selected set of warnings only, the best might be to make this parameter take a hashtable

[JamesWTruher]

I think @erich-wang has it right - shouldprocess and shouldcontinue have distinct uses. ShouldProcess is used for normally state changing operations, but ShouldContinue is set by the developer on operations which are especially sensitive (or destructive). I think our guidance is still valid and the discussion here seems to me suggests that in this case shouldcontinue should not be used. However, I'm not sure I agree. It seems that remove-azresourcegroup is a pretty destructive operation with which there is some benefit to the "belt-and-braces (suspenders)" approach. It's about providing extra confirmation on especially destructive operations.

[dcaro]

Thanks @bergmeister for taking the time to open this issue and all for the follow-up discussion. As indicated by @erich-wang this behavior has a by design aspect.

Below you describes how the implementation is done based on the current conventions commonly adopted by PowerShell that we are following for consistency and usability:

-Confirm is an automatic parameter completely handled by PowerShell that impacts the display of ShouldProcess prompts without intervention from the cmdlet.

-Force is a parameter that is defined by convention for any cmdlet that also implements ShouldContinue prompts, and avoids the ShouldContinue prompts.

Even if this article from @markcowl is a bit old, it has relevant additional details about the implementations of the confirmation prompts with Azure PowerShell: https://gist.github.com/markcowl/338e16fe5c8bbf195aff9f8af0db585d

If you want to make suggestions for changes on this implementation, the following repo is where the community can review and comment: https://github.com/PowerShell/PowerShell-RFC

Thanks, Damien - Azure PowerShell PM