Enable specifying sshd subsystem to use via -Subsystem #6603
Conversation
|
restarted mac job due to restore failure |
There was a problem hiding this comment.
I question whether we should use 'ConfigurationName' to specify the SSH PowerShell subsystem. I feel it will only confuse users where 'ConfigurationName' specifies a WinRM based PowerShell configuration. Also we would like to add session configuration to SSH in the future and should reserve the name for that. Instead we should have a new parameter name such as 'PSSubsystemName' or just 'PSSubsystem'.
|
@PaulHigin I'm ok with having it separate from |
| @@ -822,6 +823,8 @@ internal static void ValidateSpecifiedAuthentication(PSCredential credential, st | |||
| private const string KeyFilePathParameter = "KeyFilePath"; | |||
| private const string IdentityFilePathAlias = "IdentityFilePath"; | |||
| private const string PortParameter = "Port"; | |||
| private const string ConfigurationNameParameter = "ConfigurationName"; | |||
There was a problem hiding this comment.
We should remove this parameter since we don't yet support it.
There was a problem hiding this comment.
Yes, accidentally left behind
| /// <summary> | ||
| /// This parameter specifies the SSH subsystem to use for the remote connection. | ||
| /// </summary> | ||
| [Parameter(ValueFromPipelineByPropertyName = true, |
There was a problem hiding this comment.
You shouldn't need this here. This property is picked up by the base class.
| /// <summary> | ||
| /// This parameter specifies the SSH subsystem to use for the remote connection. | ||
| /// </summary> | ||
| [Parameter(ValueFromPipelineByPropertyName = true, |
There was a problem hiding this comment.
This parameter definition should go in the PSRemotingBaseCmdlet, but this appears to be in the PSExecutionCmdlet class (which derives from PSRemotingBaseCmdlet). Please move it there under the SSHHostParameters section.
There was a problem hiding this comment.
This should be the only Subsystem parameter, other cmdlets should be inheriting from this one now.
| @@ -1080,7 +1087,8 @@ private List<RemoteRunspace> CreateRunspacesForSSHHostParameterSet() | |||
| userName, | |||
| host, | |||
| this.KeyFilePath, | |||
| port); | |||
| port, | |||
| ConfigurationName); | |||
| { | ||
| ValidatePortInRange(port); | ||
|
|
||
| this.Port = (port != 0) ? port : DefaultPort; | ||
| this.Subsystem = (subsystem != null) ? subsystem : DefaultSubsystem; |
There was a problem hiding this comment.
I think we should check for null or empty here (string.IsNullOrEmpty).
| /// <summary> | ||
| /// This parameter specifies the SSH subsystem to use for the remote connection. | ||
| /// </summary> | ||
| [Parameter(ValueFromPipelineByPropertyName = true, |
There was a problem hiding this comment.
Please put this in the "SSH Parameters" section. This should also include the FilePathSSHHostParameterSet.
|
@TravisEz13 ok to merge? |
|
@SteveL-MSFT I updated the title and summary are they correct? |
SteveL-MSFT
changed the title
|
@TravisEz13 summary and title are now correct |
PR Summary
Current implementation of PSRP over SSH only enables remoting to one version of PowerShell Core 6.
sshd_config allows specifying multiple subsystems that can start different versions of PowerShell Core 6 (or same versions with different parameters).
Enable use of a new
-Subsystemparameter to specify the subsystem to use with ssh.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:to the beginning of the title and remove the prefix when the PR is ready.[feature]if the change is significant or affects feature tests