-
Notifications
You must be signed in to change notification settings - Fork 8.1k
don't needlessly pass -l login_name or -p port to ssh (PowerShell…
#11518
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
PaulHigin
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 for fixing this.
|
What's the next step required in merging this pull request? I see that @PaulHigin has approved (thanks!) but that doesn't qualify as an approving review? IOW: is there anything else you need from me? |
|
@PoshChan Please retry all |
|
@PaulHigin, successfully started retry of |
|
@LucaFilipozzi You don't need to do anything right now. It looks like help update tests all failed so I have restarted tests. The maintainer, @adityapatwardhan, still needs to approve and merge the changes. |
|
@LucaFilipozzi Thank you for your contribution! |
|
🎉 Handy links: |
PR Summary
Fix #11344 by not passing -l to ssh unless -UserName is passed to *PSSession cmdlets.
Fix #11432 by not passing -p to ssh unless -Port is passed to *PSSession cmdlets.
PR Context
When invoking *PSSession cmdlets, PowerShell's StartSSHProcess function passes a -l command line argument to the underlying operating system's ssh client even when -UserName is not passed to the cmdlets.
Similarly, StartSSHProcess passes -p to ssh even when -Port is not passed to the cmdlets.
Neither is necessary since the ssh client will determine the username from the environment and defaults the port to 22.
More importantly, by always passing -l and -p , StartSSHProcess overrides the User and Port directives in ssh_config should they have been provided by the system administrator.
This pull request should be non-breaking.
This pull request replaces #11431 which I messed up.