Add RFC: Granular Applications of ConfirmImpact

[vexx32]

No description provided.

[joeyaiello]

@vexx32 I know this is an old one, but would you mind defining the optional parameter that you're hoping to add, and how it might work in an example like the 50 machines getting Invoke-Command run against them?

Or if you don't feel this is important anymore, you can update it to Withdrawn or just close it.

[vexx32]

@joeyaiello added some more information and explanation around the method overloads, implementation example, and motivation.

Let me know if y'all need anything more 💖

[joeyaiello]

Thank you, that's a lot more helpful. We all had an aha moment on the @PowerShell/powershell-committee as soon as we saw the overload.

We're still having trouble coming up with a realistic scenario for when this would be useful. I put one on the table--checking to see if a file is system-protected on Windows as a high severity thing--but the list stopped there. Did you have more specific conditions in mind than the admittedly PoC example you showed?

Furthermore, it will be difficult for new modules to take advantage of these overloads because they won't work downlevel. For at least a couple years, the only modules that could generally take a dependency on this is stuff we ship as part of PowerShell.

[vexx32]

In my opinion, pretty much any scenario that currently is utilising a -Force parameter in combination with a $PSCmdlet.ShouldContinue() call could stand to benefit from this pattern instead. It decouples the approach from needing to use the (very overloaded and unclear in intent) -Force parameter pattern, and instead allows use of the standard common parameter -Confirm:$false to handle overrides.

I think it's less than ideal to perpetuate a pattern that demands a parameter that has no common definition / isn't a common parameter supplied from Cmdlet or PSCmdlet and is entirely optional to provide at all.

Careless authors may use ShouldContinue() without providing a corresponding -Force and as a result completely prevent certain use cases / their entire function or cmdlet from being used in any automation contexts where prompting is either not supported or not somewhere an administrator or user can actually respond to the prompt.

And yes, as with any change of this nature, it is only available after the version it's implemented in. While this is to some extent a decent argument, it's also universally true for pretty much anything we talk about adding to PowerShell. If that's one of the main limiting factors in any decision, I can't really weigh it particularly heavily -- if we preclude progress based mainly on that, we'll never get anywhere. 🙂

A possible alternative option would be to create a module that handles this instead and expose it as cmdlets, e.g., Use-ConfirmImpact -High { <# script #> } or something along those lines. Depending on the implementation specifics, it's possible that will end up being more compatible / available to earlier versions of PowerShell.

IMO the API could use some improvement going forward in any case, which is why I proposed the improvement to the API itself first 🙂

[iSazonov]

This would be nice to have but do we really ready to review most of popular cmdlets/modules and make many breaking changes to implement the RFC? Without this the RFC will be die RFC.

[vexx32]

@iSazonov at least in the beginning no breaking changes are necessary. -Force parameters can be hidden but retained and their functionality essentially copied into the conditionals to allow old code and new to work, at least for the most part.

Something like this would be perfectly fine:

if (Force.IsPresent || ShouldProcess(target, actionString, ConfirmImpact.High))
{
    // Replaces the functionality of the old ShouldContinue() pattern.
}

[joeyaiello]

@vexx32: we had a discussion with the @PowerShell/powershell-committee on this one today.

We agree with the issues that you raised with $force, namely that it becomes overloaded within a particular cmdlet, specifically one that needs multiple confirmation prompts within different scenarios. Unfortunately, this problem still exists with $confirm:false because that will affect every type of confirmation prompt in the cmdlet (though it's slightly more limited in scope because -Force gets used for additional situations where confirmation prompts aren't thrown, like deleting files with certain ACLs).

One proposal we floated to solve this problem differently is to use $PSCmdlet.ShouldContinue in conjunction with a parameter other than -Force.

For more info, see this example:

function set-thing {     
  [CmdletBinding(SupportsShouldProcess=$true)]
  param ( [switch]$noask ) 
  process {
    if ( $noask -or $PSCmdlet.ShouldContinue("foo","bar")) {
      write-host "DO IT NOW"
    } else {
      write-host "Don't Do It"
    }
  }
}

Unfortunately, this precludes the ability to easily bypass all confirmation prompts within an automation scenario. So, one alternate proposal may be to make a breaking change to ShouldContinue so that it respects $ConfirmPreference. That way, users can set $ConfirmPreference = 'None' in automation scenarios without having to know which individual parameter is the "bypass" parameter for a given cmdlet.

If none of this scans for you, it'd still be really great to see an experiment of this within its own module on the Gallery so that we can see if others have interest in this scenario.

If you're interested in any of these additional proposals and/or you implement said module and believe it has wide-reaching utility, then you should open a new issue to discuss this in more depth over at PowerShell/PowerShell. For the time being, given the lack of interest in this particular RFC, we're going to close it.

[vexx32]

Fair enough!

I think the issue of overloading -Force is mainly a problem where the meaning is unclear and cannot be easily inferred. That can often be solved by simply renaming the parameter.

In my mind, ShouldContinue() is an oft-misused API, primarily because of the issues with automation use cases it creates, but I don't think there's a lot of value in making it work more like ShouldProcess() -- we already have that. My thought is that the general guidance should probably just be to have folks use ShouldProcess(), and if they want to add a -Force style parameter to override that, they can just do that as well. It would also tie in better with setting $ConfirmPreference for CI / automation purposes.

Were it up to me, I would probably just remove ShouldContinue() from any documentation that currently recommends its use and replace it with ShouldProcess(), or perhaps relegate it to a footnote, being careful to note its incompatibility with automation scenarios and likely limited use case as a result.

It's an API that maybe in some cases is necessary, but I think on the whole it tends to be more trouble than it's worth to use it in a way that doesn't make the tool you're writing non-functional in an automation context. Perhaps some notes to that effect in its documentation would go a good way to helping folks pick the right options for what they intend their tools to be used for.