-
Notifications
You must be signed in to change notification settings - Fork 8.1k
Add error for Get-PSSession -ComputerName on Unix #21009
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
Conversation
694134b to
b5767a0
Compare
|
There was WG conclusion about root of the issue #6321 (comment) |
I think Paul may have been confused there, |
|
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
|
Hmm, I see Paul said that we can enumerate any |
|
Yep and using -ComputerName is for sessions on a completely different host. In fact it uses a mechanism in the WSMan service and not PowerShell itself to track the sessions on the remote host. Maybe how PowerShell tracks that information can be expanded in the future but it shouldn’t hold up something like this. |
|
@jborean93 For my education, does |
|
When you do $getParams = @{
ResourceURI = 'Shell'
Enumerate = $true
ComputerName = 'foo'
SessionOption = (New-WSManSessionOption)
}
Get-WSManInstance @getParamsIn protocol terms this sends the following WSMan POST message to <s:Envelope xmlns:s="http://www.w3.org/2003/05/soap-envelope" xmlns:a="http://schemas.xmlsoap.org/ws/2004/08/addressing" xmlns:n="http://schemas.xmlsoap.org/ws/2004/09/enumeration" xmlns:w="http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd" xmlns:p="http://schemas.microsoft.com/wbem/wsman/1/wsman.xsd" xmlns:b="http://schemas.dmtf.org/wbem/wsman/1/cimbinding.xsd">
<s:Header>
<a:To>http://foo:5985/wsman</a:To>
<w:ResourceURI s:mustUnderstand="true">http://schemas.microsoft.com/wbem/wsman/1/windows/shell</w:ResourceURI>
<a:ReplyTo>
<a:Address s:mustUnderstand="true">http://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymous</a:Address>
</a:ReplyTo>
<a:Action s:mustUnderstand="true">http://schemas.xmlsoap.org/ws/2004/09/enumeration/Enumerate</a:Action>
<w:MaxEnvelopeSize s:mustUnderstand="true">512000</w:MaxEnvelopeSize>
<a:MessageID>uuid:B03C7765-66AC-4DAB-B84D-BC62464A1AC8</a:MessageID>
<w:Locale xml:lang="en-US" s:mustUnderstand="false"/>
<p:DataLocale xml:lang="en-US" s:mustUnderstand="false"/>
<p:SessionId s:mustUnderstand="false">uuid:00DC848B-B247-485C-B144-EB709AA1586A</p:SessionId>
<p:OperationID s:mustUnderstand="false">uuid:CB4C0368-D3A8-45D0-AE6E-2C1B90FAD70E</p:OperationID>
<p:SequenceId s:mustUnderstand="false">1</p:SequenceId>
<w:OperationTimeout>PT60.000S</w:OperationTimeout>
</s:Header>
<s:Body>
<n:Enumerate>
<w:OptimizeEnumeration/>
<w:MaxElements>32000</w:MaxElements>
</n:Enumerate>
</s:Body>
</s:Envelope>This <s:Envelope xmlns:s="http://www.w3.org/2003/05/soap-envelope" xmlns:a="http://schemas.xmlsoap.org/ws/2004/08/addressing" xmlns:n="http://schemas.xmlsoap.org/ws/2004/09/enumeration" xmlns:w="http://schemas.dmtf.org/wbem/wsman/1/wsman.xsd" xmlns:p="http://schemas.microsoft.com/wbem/wsman/1/wsman.xsd" xml:lang="en-US">
<s:Header>
<a:Action>http://schemas.xmlsoap.org/ws/2004/09/enumeration/EnumerateResponse</a:Action>
<a:MessageID>uuid:142D87D6-7CBC-4E9E-9D4F-95CD0EE5CAA2</a:MessageID>
<a:To>http://schemas.xmlsoap.org/ws/2004/08/addressing/role/anonymous</a:To>
<p:OperationID s:mustUnderstand="false">uuid:CB4C0368-D3A8-45D0-AE6E-2C1B90FAD70E</p:OperationID>
<p:SequenceId>1</p:SequenceId>
<a:RelatesTo>uuid:B03C7765-66AC-4DAB-B84D-BC62464A1AC8</a:RelatesTo>
</s:Header>
<s:Body>
<n:EnumerateResponse>
<n:EnumerationContext/>
<w:Items>
<rsp:Shell xmlns:rsp="http://schemas.microsoft.com/wbem/wsman/1/windows/shell">
<rsp:ShellId>E578F8FB-5A82-4491-AC46-E04EDBD41CD7</rsp:ShellId>
<rsp:Name>WinRM1</rsp:Name>
<rsp:ResourceUri>http://schemas.microsoft.com/powershell/Microsoft.PowerShell</rsp:ResourceUri>
<rsp:Owner>DOMAIN\vagrant-domain</rsp:Owner>
<rsp:ClientIP>192.168.56.12</rsp:ClientIP>
<rsp:ProcessId>832</rsp:ProcessId>
<rsp:IdleTimeOut>PT7200.000S</rsp:IdleTimeOut>
<rsp:InputStreams>stdin pr</rsp:InputStreams>
<rsp:OutputStreams>stdout</rsp:OutputStreams>
<rsp:MaxIdleTimeOut>PT2147483.647S</rsp:MaxIdleTimeOut>
<rsp:Locale>en-US</rsp:Locale>
<rsp:DataLocale>en-US</rsp:DataLocale>
<rsp:CompressionMode>XpressCompression</rsp:CompressionMode>
<rsp:ProfileLoaded>Yes</rsp:ProfileLoaded>
<rsp:Encoding>UTF8</rsp:Encoding>
<rsp:BufferMode>Block</rsp:BufferMode>
<rsp:State>Connected</rsp:State>
<rsp:ShellRunTime>P0DT0H2M29S</rsp:ShellRunTime>
<rsp:ShellInactivity>P0DT0H2M29S</rsp:ShellInactivity>
<rsp:MemoryUsed>62MB</rsp:MemoryUsed>
<rsp:ChildProcesses>0</rsp:ChildProcesses>
</rsp:Shell>
<rsp:Shell xmlns:rsp="http://schemas.microsoft.com/wbem/wsman/1/windows/shell">
<rsp:ShellId>D76BA8D1-C52A-4E10-8A28-46262426AC09</rsp:ShellId>
<rsp:Name>Runspace1</rsp:Name>
<rsp:ResourceUri>http://schemas.microsoft.com/powershell/PowerShell.7</rsp:ResourceUri>
<rsp:Owner>DOMAIN\vagrant-domain</rsp:Owner>
<rsp:ClientIP>192.168.56.12</rsp:ClientIP>
<rsp:ProcessId>6900</rsp:ProcessId>
<rsp:IdleTimeOut>PT7200.000S</rsp:IdleTimeOut>
<rsp:InputStreams>stdin pr</rsp:InputStreams>
<rsp:OutputStreams>stdout</rsp:OutputStreams>
<rsp:MaxIdleTimeOut>PT2147483.647S</rsp:MaxIdleTimeOut>
<rsp:Locale>en-US</rsp:Locale>
<rsp:DataLocale>en-US</rsp:DataLocale>
<rsp:CompressionMode>XpressCompression</rsp:CompressionMode>
<rsp:ProfileLoaded>Yes</rsp:ProfileLoaded>
<rsp:Encoding>UTF8</rsp:Encoding>
<rsp:BufferMode>Block</rsp:BufferMode>
<rsp:State>Connected</rsp:State>
<rsp:ShellRunTime>P0DT0H2M18S</rsp:ShellRunTime>
<rsp:ShellInactivity>P0DT0H2M16S</rsp:ShellInactivity>
<rsp:MemoryUsed>97MB</rsp:MemoryUsed>
<rsp:ChildProcesses>1</rsp:ChildProcesses>
</rsp:Shell>
</w:Items>
<w:EndOfSequence/>
</n:EnumerateResponse>
</s:Body>
</s:Envelope>In this case Ultimately it's all sessions managed by the remote service not just ones opened by the current process/computer. This is why |
|
Thanks for the full answer! I believe the user must have the appropriate permissions for WSMan to return the list of sessions. In this case, the user could connect to the remote computer via SSH, get a list of all pwsh processes and even connect to them via local socket. Maybe this is what Paul meant. |
It certainly needs permission to perform the
Potentially, honestly I don't see |
Eh, this will definitely be in demand for the management tool that is PowerShell. If WSMan does this, then users will expect the same from any custom transport that replaces WSMan. It would be better for UX if the base cmdlets didn't change and were independent of transport. |
|
I honestly feel like PSRemoting is inferior in pretty much most circumstances for interactive use which is where you typically want disconnection support. Using pure ssh with tools like tmux offer such a better experience and will be what I typically recommend going forward. Someone may want to reimplement disconnection support for custom transports but it’ll require a lot of work and an even more special server side protocol to make use of it. Until that time I think it’s safe to just make this functionality Windows only. |
|
@SteveL-MSFT it's been over 3 months without any review by someone from the PowerShell team. Any chance this can be added to the next cycle. |
9ae516f to
70dc4cb
Compare
70dc4cb to
a413bf8
Compare
Emit an exception when attempting to use Get-PSSession -ComputerName on a non-Windows platform that makes it clear it cannot be used on those platforms.
a413bf8 to
186a173
Compare
iSazonov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jborean93 Please add one test for Unix.
I believe we can merge (after you address my requests) since it is obvious improvement.
| <value>Out of process memory.</value> | ||
| </data> | ||
| <data name="UnsupportedOSForRemoteEnumeration" xml:space="preserve"> | ||
| <value>Remote PSSession enumeration with -ComputerName is only supported on Windows and not "{0}".</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes no sense to point OS version. Please remove:
| <value>Remote PSSession enumeration with -ComputerName is only supported on Windows and not "{0}".</value> | |
| <value>Remote PSSession enumeration with -ComputerName is only supported on Windows.</value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it provides a few extra debug details and doesn't hurt to include. It's also somewhat helpful to identify where the cmdlet was run when it comes to remoting ones as people may be confused if the OS platform is local or remote based. Including the local OS information in the error helps to avoid that ambiguity.
| { | ||
| base.BeginProcessing(); | ||
| #if UNIX | ||
| if (ComputerName?.Length > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our common pattern is to check whether a parameter presents.
if (MyInvocation.BoundParameters.ContainsKey(nameof(ComputerName)))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a particular reason for this. The code uses the ComputerName property value and thus it makes more sense to do the check on the actual property rather than a pwsh-ism here. It's also cleaner and shorter than accessing a dictionary to check if a key exists vs just checking the property value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah there are situations where it's better but I think it's fine here.
SeeminglyScience
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks Jordan!
|
📣 Hey @jborean93, how did we do? We would love to hear your feedback with the link below! 🗣️ 🔗 https://aka.ms/PSRepoFeedback |
PR Summary
Emit an exception when attempting to use Get-PSSession -ComputerName on a non-Windows platform that makes it clear it cannot be used on those platforms. Now when running this on non-Windows it will emit the following error.
PR Context
I don't think this feature will ever be implemented, even if it is it would be nice to error with a proper message not the current
Fixes: #20995
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).