Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Populate FunctionsToExport with explicit list of commands #205

Open
FriedrichWeinmann opened this issue Jan 22, 2024 · 5 comments
Open

Populate FunctionsToExport with explicit list of commands #205

FriedrichWeinmann opened this issue Jan 22, 2024 · 5 comments
Labels
question This is a general question.

Comments

@FriedrichWeinmann
Copy link

Hi,

thanks for the awesome project! I've got a small feature request:
Could you - going forward - update your PSPKI.psd1 on release to no longer include FunctionsToExport = '*' and instead list each command individually?
The wildcard is convenient as you develop, but it has some problems when trying to run the module in a hardened environment and it makes discovery a pain.

For example, I can search for all modules on the gallery that contain a given command:

Find-Module -Command Revoke-Certificate

As of now, there's one module that's not going to be found ...

@Crypt32 Crypt32 added the question This is a general question. label Jan 23, 2024
@Crypt32
Copy link
Collaborator

Crypt32 commented Jan 23, 2024

The module determines exported commands in runtime depending on client capabilities to avoid exporting functions that technically cannot be executed in a particular session. As the result, the list of exported functions is dynamic and cannot be statically populated in .psd1 file.

@FriedrichWeinmann
Copy link
Author

Would it not be more useful then to publish all commands and fail the ones whose requirements are not met?
As of now, running a command on a incompatible client would result in a "Command not found" error, rather than a "Bad Client, do XYZ to fix" error.
I can't count the number of times I've had to help an admin with Exchange Tooling where they were looking for the correct version when it was just a matter of insufficient permissions (EX/EXO only give you the commands you have permissions to use).

@hmiller10
Copy link

Exporting functions with an '*' in the manifest file will cause PowerShell not recognize the function if PowerShell is running in constrained language mode. Each function must be explicitly listed as is currently being done.

@Crypt32
Copy link
Collaborator

Crypt32 commented Jan 24, 2024

Would it not be more useful then to publish all commands and fail the ones whose requirements are not met?

That's what I wanted to avoid. Otherwise, I will have to write platform/dependency support check in every function and this will add confusion: the command exist, but cannot run.

@FriedrichWeinmann
Copy link
Author

I'd argue on the confusion part - not whether it will happen, but which of the two possible points of confusion are worse (since you don't get to pick an option that is 100% confusion free for everybody).

  • Command Exists but cannot run: The problem is in the environment, something an admin is likely to understand (can't manage AD-based CA without AD is understandable)
  • Command exists not: PowerShell specific issue, nothing about CA

Given that the target audience is likely to be CA admins rather than PowerShell experts, I believe that the "Command cannot be used" problem is a lot easier to explain or understand.

As for the implementation headache - I strongly recommend implementing assertions for that kind of situation:

One internal function per environment condition:

function Assert-Something {
	<#
        .SYNOPSIS
                Assert that something is true.

        .DESCRIPTION
                Assert that something is true.

        .PARAMETER Cmdlet
                The $PSCmdlet variable of the calling command.

        .EXAMPLE
                PS C:\> Assert-Something -Cmdlet $PSCmdlet

                Assert that something is true.
#>
	[CmdletBinding()]
	param (
		[Parameter(Mandatory = $true)]
		$Cmdlet
	)

	process {
		if ("Something" -is $true) {
			return
		}

		$exception = [System.InvalidOperationException]::new("Invalid environment! This command can only run if XYZ is true!")
		$record = [System.Management.Automation.ErrorRecord]::new($exception, "InvalidEnvironment", [System.Management.Automation.ErrorCategory]::InvalidOperation, $null)
		$Cmdlet.ThrowTerminatingError($record)
	}
}

First statement in the begin block of each function thus constrained:

Assert-Something -Cmdlet $PSCmdlet

With that you have one central place where you implement both code and error message, the actual impact on your individual commands' complexity is minimal. The internas are hidden from the user and you can use the error message to usefully complain what is the issue, so the confusion should be minimal.

Thank you for taking the time on this issue, whether I managed to convince you or not (it is a matter of perspectives and priorities, so I'm hardly going to judge either way).
Your work on this project is appreciated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question This is a general question.
Projects
None yet
Development

No branches or pull requests

3 participants