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.

Sign up for GitHub

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

Jump to bottom

Add none support to ProxyJump #893

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Add none support to ProxyJump #893

wants to merge 5 commits into from

Conversation

elmobp
Copy link

@elmobp elmobp commented Jan 17, 2023

As per OpenSSH man docs

ProxyJump
             Specifies one or more jump proxies as either
             [user@]host[:port] or an ssh URI.  Multiple proxies may be
             separated by comma characters and will be visited
             sequentially.  Setting this option will cause ssh(1) to
             connect to the target host by first making a ssh(1)
             connection to the specified ProxyJump host and then
             establishing a TCP forwarding to the ultimate target from
             there.  Setting the host to none disables this option
             entirely.

This along with the tests should obey the above

mfazekas
mfazekas reviewed Jan 24, 2023
View reviewed changes
lib/net/ssh/config.rb Outdated
@@ -154,7 +154,10 @@ def load(path, host, settings = {}, base_dir = nil)
# ProxyCommand and ProxyJump override each other so they need to be tracked togeather
%w[proxyjump proxycommand].each do |proxy_key|
if (proxy_value = settings.delete(proxy_key))
settings['proxy'] ||= [proxy_key, proxy_value]
# When ProxyJump is set to none explicity disable the jumphost
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we clear settings['proxy'] if proxy jump is none. I mean if one config has a proxy configured but a more specific sets it none then we should clear it, shouldn't we?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I totally missed this! Let me fix that up

mfazekas
mfazekas reviewed Mar 26, 2023
View reviewed changes
lib/net/ssh/config.rb Outdated Show resolved Hide resolved
@elmobp @mfazekas
Update lib/net/ssh/config.rb
84d4172 
Co-authored-by: Miklós Fazekas <mfazekas@szemafor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfazekas mfazekas mfazekas left review comments

Assignees
No one assigned
Labels
Needs: Author Feedback
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@elmobp @mfazekas