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

Do not set by default SSH port when using Net::OpenSSH (fix: 1495) #1496

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nfetisov
Copy link

(R)?ex can't parse complex OpenSSH configuration file,
i.e. with host templates, includes, etc.
Net::OpenSSH use OpenSSH library and handle complex .ssh/config
right like OpenSSH itself.

So if (R)?ex can find port for SSH connection inside
(simple plain) .ssh/config - let's use this port value,
and if it can't - do not force port to 22/tcp so Net::OpenSSH
can try to find it by itself (or failover to the same 22/tcp
inside Net::OpenSSH).

(R)?ex can't parse complex OpenSSH configuration file,
i.e. with host templates, includes, etc.
Net::OpenSSH use OpenSSH library and handle complex .ssh/config
right like OpenSSH itself.

So if (R)?ex can find port for SSH connection inside
(simple plain) .ssh/config - let's use this port value,
and if it can't - do not force port to 22/tcp so Net::OpenSSH
can try to find it by itself (or failover to the same 22/tcp
inside Net::OpenSSH).
Copy link
Member

@ferki ferki left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

This PR does not fully follow the project's expectations, so I'd like to take this chance to kindly link our Contributing guide, specifically the section about GitHub issues and pull requests.

Please follow the pull request template to provide all expected information either by editing the opening post here, or by opening a new pull request instead of this one.


This PR seems to be missing an expected changelog entry, and also failed the automated tests.

The changes here are overall on the right track towards letting Net::OpenSSH directly use the ssh config from outside Rex (as, IMHO, it always should have been). As commented on the related issue, it's only a part of the full picture, so I'd say it's unlikely that we would be able to merge it in its current form.

I'll keep this open a bit more for reference, probably until I come up with a more complete solution of the root cause.

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

Successfully merging this pull request may close these issues.

2 participants