Config file processing - takes in config_file from custom path by KB-perByte · Pull Request #790 · ansible/pylibssh

KB-perByte · 2026-01-28T05:29:01Z

SUMMARY

This patch adds support for loading and applying settings from configuration files to libssh connection sessions.

ISSUE TYPE

Bugfix Pull Request

ADDITIONAL INFORMATION

ansible-collections/ansible.netcommon#430

#env:
$ python -c "from pylibsshext import __full_version__; print(__full_version__)"
<pylibsshext v0.1.dev2067+g570153995.d20260213 with libssh v0.12.0>

# ../../custom_ssh_file
Host cisco-router-direct-demo
    HostName 54.1c.cx.yx
    User cisco
    Port 0000

# ./inventory
[ios]
test

[ios:vars]
ansible_host=cisco-router-direct-demo
ansible_network_os=cisco.ios.ios
ansible_ssh_password=TOPSECRETPASSWORD
ansible_connection=ansible.netcommon.network_cli
ansible_network_cli_ssh_type=libssh
ansible_libssh_config_file=../../custom_ssh_file

# ./Playbook.yaml
---
- hosts: ios
  gather_facts: false
  tasks:
    - name: Just connect and gather
      cisco.ios.ios_hostname:
        config:
        state: gathered

log to prove the change

2026-02-13 12:28:01,946 p=95788 u=batmon n=ansible INFO| <cisco-router-direct-demo> local domain socket path is /home/batmon/.ansible/pc/83b7a917df
2026-02-13 12:28:01,946 p=95788 u=batmon n=ansible INFO| <cisco-router-direct-demo> ANSIBLE_NETWORK_IMPORT_MODULES: enabled
2026-02-13 12:28:02,188 p=95788 u=batmon n=ansible INFO| <cisco-router-direct-demo> ANSIBLE_NETWORK_IMPORT_MODULES: found cisco.ios.ios_hostname at /home/batmon/Work/AnsibleNetwork/collections/ansible_collections/cisco/ios/plugins/modules/ios_hostname.py
2026-02-13 12:28:02,188 p=95788 u=batmon n=ansible INFO| <cisco-router-direct-demo> ANSIBLE_NETWORK_IMPORT_MODULES: running cisco.ios.ios_hostname
2026-02-13 12:28:02,188 p=95788 u=batmon n=ansible INFO| <cisco-router-direct-demo> ANSIBLE_NETWORK_IMPORT_MODULES: _load_params skipped for action plugin in direct execution
2026-02-13 12:28:02,189 p=95847 u=batmon n=ansible INFO| jsonrpc request: b'{"jsonrpc": "2.0", "method": "get_capabilities", "id": "daa57e56-b8f8-4fd5-9c0c-a5e7e69c0cc9", "params": [[], {}]}'
2026-02-13 12:28:02,189 p=95847 u=batmon n=ansible INFO| <cisco-router-direct-demo> USING PYLIBSSH VERSION 0.1.dev2067+g570153995.d20260213
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible INFO| <cisco-router-direct-demo> ESTABLISH LIBSSH CONNECTION FOR USER: None TO cisco-router-direct-demo (port from SSH config)
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_config_parse_file: Reading configuration data from /redacted/..//ssh_config
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_connect: libssh 0.11.3 (c) 2003-2025 Aris Adamantiadis, Andreas Schneider and libssh contributors. Distributed under the LGPL, please refer to COPYING file for information about your rights, using threading threads_pthread
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible-pylibssh DEBUG| getai: host 54.1c.cx.yx matches an IP address
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_socket_connect: Nonblocking connection socket: 7
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_connect: Socket connecting, now waiting for the callbacks to work
2026-02-13 12:28:02,190 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_connect: Actual timeout : 2000000
2026-02-13 12:28:02,485 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_socket_pollcallback: Received POLLOUT in connecting state
2026-02-13 12:28:02,485 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_client_connection_callback: session_state=2
2026-02-13 12:28:02,485 p=95847 u=batmon n=ansible-pylibssh DEBUG| ssh_socket_unbuffered_write: Enabling POLLOUT for socket
2026-02-13 12:28:02,868 p=95847 u=batmon n=ansible-pylibssh DEBUG| callback_receive_banner: Received banner: SSH-2.0-Cisco-1.25

KB-perByte · 2026-02-13T18:30:39Z

The CI is passing at this point!

Jakuje

looks good!
(reviewing just the config part as it includes also the logging part that was actually written by me in separate PR)

ganeshrn · 2026-02-14T00:18:11Z

@KB-perByte Is this variable OPENSSH_PROXYJUMP required to be passed as a task environment variable if yes we might have to update it as part of porting documentation from paramiko.

Jakuje · 2026-02-14T08:40:14Z

@KB-perByte Is this variable OPENSSH_PROXYJUMP required to be passed as a task environment variable if yes we might have to update it as part of porting documentation from paramiko.

The OPENSSH_PROXYJUMP should not be passed by default. We have now libssh 0.12 which should fix some corner cases of proxyjump handling in libssh so it would be better not to use the workaround with the variable and rather report if there will be some outstanding issues.

The OPENSSH_PROXYJUMP just makes libssh call openssh for proxyjumps, which is not ideal.

KB-perByte · 2026-02-16T05:07:22Z

@ganeshrn, this PR is more concerned about the config file handling from a custom path, I am testing things with the newly released version of libssh, which should do justice to JumpHost configurations from configFiles.

And as I have a +1 from @Jakuje for his changes for logging here in this PR, I'll update the changelog to reflect the same and get this merged.

Thanks for the reviews.

Jakuje · 2026-02-16T10:02:53Z

Still ack. But the history now looks quite messy and is mixing 3 different things -- parsing of configuration file, logging and proxyjump. I would prefer to get the #597 merged separately (just rebased) and then rebase this + squash the fixup commits.

KB-perByte · 2026-02-16T12:04:31Z

+1 I'll hold on to this PR, and rebase it with #597
@Jakuje

webknjaz · 2026-02-16T13:20:10Z

Is this related to #479?

KB-perByte · 2026-02-16T16:47:21Z

converting this PR back to draft and removing logging specific changes out of this PR
and focus on this fix only #479

#597 to be considered for merge!

webknjaz · 2026-02-17T15:00:35Z

and focus on this fix only #479

@KB-perByte perhaps, just polish that PR? Or pull in their commits? Or if you go with this one, do make sure to implement https://hynek.me/til/easier-crediting-contributors-github/.

webknjaz · 2026-02-25T17:40:31Z

@KB-perByte try to use rebasing instead of adding merge commits into topic branches since that leads to foxtrot merges in the tree.

- Add parse_config() to Session for parsing ~/.ssh/config or custom config files - Expose ssh_options_parse_config in libssh.pxd - Changelog fragment for config file support (fixes ansible#790) Made-with: Cursor

webknjaz · 2026-03-03T13:50:53Z

+        )
+
+
+def test_connect_config_file_none_no_parse():


Seems like test_parse_config_nonexistent_raises is practically the same and they could be merged though parametrize with different error + host + file in params.

webknjaz · 2026-03-03T13:54:50Z

        # Parse SSH config after host is set (so Host blocks match) but before connect,
        # so that options from the config (e.g. HostName, User, Port, ProxyCommand) are
-        # applied to this connection. Only when config_file is passed explicitly.
-        with _ctx.suppress(KeyError):


Unclear why you reverted this — the CM bit was good, the None double-check still isn't.

webknjaz

I'll wait until here's responses to the questions I'd already asked before I leave any more feedback.

UPD (06.03.2026): @KB-perByte and I had a 1:1 call the other day to clear some things up and synchronize the understanding. This is now waiting for his changes which will likely happen next week as I believe we're on the same page now. Meanwhile, I'll finish up #808 and cut a release right after (hopefully today!) to make the logging improvements available early.

webknjaz

I did another quick pass and realized that another thing I mentioned earlier somewhere hasn't been followed up — there's an earlier PR #479 that somebody took time to file. Even though it remains unmerged, let's be nice and credit the contributor.

Inline, I've left change note-specific instructions but it'd be good to add a co-authored-by trailer to the commit changing libssh.pxd following https://hynek.me/til/easier-crediting-contributors-github/.

webknjaz · 2026-03-06T15:06:11Z

+``Host`` aliases, ``ProxyCommand``, or per-host settings) before the connection is
+established. See :ref:`Connecting with a custom SSH config <custom-ssh-config>` for examples.
+
+-- by :user:`KB-perByte`


Since this includes bits of #479, let's remember to credit the contributor:

Suggested change

-- by :user:`KB-perByte`

-- by :user:`donnerhacke` and :user:`KB-perByte`

webknjaz · 2026-03-06T15:10:17Z

Same as with the byline, add a symlink mentioning the other PR:

$ (cd docs/changelog-fragments/ && ln -sf {790,479}.feature.rst && git add 479.feature.rst) $ git commit -m '📝 Link PR #479 to PR #790 change note'

webknjaz · 2026-03-06T15:16:58Z



+cdef class LibsshConfigParseException(LibsshSessionException):
+    """Raised when SSH config file parsing fails."""


Just realized that this docstring references some external logic that may change over time in a disconnected manner. That behavior description doesn't belong here.
The docstring should instead explain what the exception represents. How about

Suggested change

"""Raised when SSH config file parsing fails."""

"""A failure to parse or load an SSH configuration file."""

?

webknjaz · 2026-03-06T15:19:26Z

+        if path is not None:
+            self.parse_config(path)


Why can't this be

Suggested change

if path is not None:

self.parse_config(path)

self.parse_config(path)

/

Suggested change

if path is not None:

self.parse_config(path)

self.load_config_into_session(path)

/

Suggested change

if path is not None:

self.parse_config(path)

self._parse_config(path)

/

Suggested change

if path is not None:

self.parse_config(path)

self._load_config_into_session(path)

?

for more information, see https://pre-commit.ci

KB-perByte · 2026-03-11T17:41:22Z

@webknjaz Ithe CI is not stable, I'll get back to this PR again later. The failures are unrelated.
I could address most of the revirew comments.

Jakuje · 2026-03-11T20:47:28Z

https://github.com/ansible/pylibssh/actions/runs/22965988172/job/66671296544?pr=790

urllib.error.HTTPError: HTTP Error 429: Too Many Requests

sounds like the downloads are failing due to some rate limiting ...

But the failure in the following pipeline sounds related:

https://github.com/ansible/pylibssh/actions/runs/22965988172/job/66671296430?pr=790#step:6:802

/project/tests/unit/session_test.py:54: in
path_arg_type: type[bytes] | type[str] | type[Path],
E TypeError: unsupported operand type(s) for |: 'types.GenericAlias' and 'types.GenericAlias'

The error in https://github.com/ansible/pylibssh/actions/runs/22965988172/job/66671296510?pr=790 sounds related to changes in this PR too:

src/pylibsshext/session.pyx:562:71: Expected an identifier, found '/'

KB-perByte · 2026-03-12T06:14:01Z

@Jakuje checking the same, Cython doesn't support PEP 570's positional-only parameter syntax (the / in the function signature) getting em fixed

KB-perByte · 2026-03-12T06:33:41Z

https://github.com/ansible/pylibssh/actions/runs/22989405548/job/66747015702?pr=790#step:6:802 I am not 100% sure what would the right way to fix these is, checking for the possible fixes.


  ==================================== ERRORS ====================================
  _________________ ERROR collecting tests/unit/session_test.py __________________
  /project/tests/unit/session_test.py:54: in <module>
      path_arg_type: type[bytes] | type[str] | type[Path],
  E   TypeError: unsupported operand type(s) for |: 'types.GenericAlias' and 'types.GenericAlias'
          LibsshConfigParseException = <class 'pylibsshext.errors.LibsshConfigParseException'>
          LibsshSessionException = <class 'pylibsshext.errors.LibsshSessionException'>
          Path       = <class 'pathlib.Path'>
          Session    = <class 'pylibsshext.session.Session'>
          __builtins__ = <builtins>
          __cached__ = '/project/tests/unit/__pycache__/session_test.cpython-39.pyc'
          __doc__    = 'Tests suite for session.'
          __file__   = '/project/tests/unit/session_test.py'
          __loader__ = <_pytest.assertion.rewrite.AssertionRewritingHook object at 0x7fca12a26ca0>
          __name__   = 'session_test'
          __package__ = ''

for more information, see https://pre-commit.ci

webknjaz · 2026-03-12T14:55:40Z

add annotations for 2.9

What's this mysterious "2.9"?

webknjaz · 2026-03-12T15:04:00Z

 %global canonical_project_name ansible-pylibssh
 %global pypi_name %canonical_project_name
-%global normalized_core_packaging_metadata_project_name %(echo '%canonical_project_name' | sed --regexp-extended 's:-:_:g;s:\\.:_:g')
+%global normalized_core_packaging_metadata_project_name %(echo '%canonical_project_name' | sed --regexp-extended 's:-:_:g;s:[.]:_:g')


It's possible that the fix will only be needed on this line. The error was caused by c94f365 somehow, it seems, not sure why. But line 8 only got changes to the var name so it's unlikely that it needs changing. I'll look into it further — it's surprising that my smoke test script doesn't fail on this locally, nor do the GHA jobs, it seems — looks like Packit might have different versions of lua or something in its env compared to the regular Fedora and UBI images.

webknjaz · 2026-03-12T15:05:42Z

+
+# Alternative: call parse_config() explicitly (e.g. with a pathlib.Path)
+# ssh = Session(host=HOST)
+# ssh.parse_config(Path(CONFIG_FILE))  # or str/bytes


Why do you insist on bytes of unguaranteed encoding in the API?

webknjaz · 2026-03-12T15:07:11Z

+        if host is not None:
+            self.set_ssh_options('host', host)


This looks new.. How is it related?

this looks like the host argument was never handled through the kwargs as the rest of the options. It should land likely at least as a separate bugfix commit, I think.

Yeah, that's what I thought. A separate bug rather than a feature which this one is. And a separate change note.

webknjaz · 2026-03-12T15:08:32Z

+    ids=str,
+)
+def test_parse_config_valid_file_succeeds(
+    path_arg_type: type[bytes | str | Path],


This doesn't sound right. It shouldn't be a type of a union but a union of types, I think. Also, the bytes controversy in the API stands.

webknjaz · 2026-03-12T19:46:38Z

I'm not seeing this surfacing in other PRs. So just drop the commit that added it when rebasing the next time.

webknjaz · 2026-03-13T12:48:47Z

FYI, I'm looking at adding linting for requiring not creating foxtrot merges automatically.

codecov · 2026-03-20T18:07:21Z

Codecov Report

❌ Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.82%. Comparing base (b896c8e) to head (4781f5e).

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

for more information, see https://pre-commit.ci

KB-perByte · 2026-03-20T18:20:43Z

Apologies for the delay in activity. I am trying to progress multiple items in various repos.
It takes me quite some time to get back to review comments.

We would need to prioritize this PR, to get the broken functionality fixed wrt to netcommon.
We can have another iteration of the fixes if those are not must-have.

KB-perByte marked this pull request as draft January 28, 2026 05:30

This comment was marked as outdated.

Sign in to view

KB-perByte changed the title ~~WIP Config file processing~~ Config file processing - takes in config_file from custom path Feb 11, 2026

KB-perByte requested a review from ganeshrn February 11, 2026 12:00

This comment was marked as resolved.

Sign in to view

psf-chronographer Bot added the bot:chronographer:provided There is a change note present in this PR label Feb 13, 2026

KB-perByte marked this pull request as ready for review February 13, 2026 06:53

KB-perByte requested a review from Jakuje February 13, 2026 06:53

KB-perByte requested a review from ganeshrn February 13, 2026 18:40

Jakuje approved these changes Feb 13, 2026

View reviewed changes

ganeshrn approved these changes Feb 14, 2026

View reviewed changes

webknjaz reviewed Feb 16, 2026

View reviewed changes

Comment thread docs/changelog-fragments/790.bugfix.rst Outdated

KB-perByte marked this pull request as draft February 16, 2026 16:47

KB-perByte force-pushed the config_file branch from 4eafd5d to c127a84 Compare February 26, 2026 11:14

KB-perByte marked this pull request as ready for review February 26, 2026 11:21

Add SSH config file parsing support

0cdac3a

- Add parse_config() to Session for parsing ~/.ssh/config or custom config files - Expose ssh_options_parse_config in libssh.pxd - Changelog fragment for config file support (fixes ansible#790) Made-with: Cursor

KB-perByte force-pushed the config_file branch from 2f7b265 to 0cdac3a Compare February 26, 2026 11:35

KB-perByte requested review from Jakuje, ganeshrn and webknjaz February 26, 2026 12:07

webknjaz reviewed Mar 3, 2026

View reviewed changes

webknjaz requested changes Mar 3, 2026

View reviewed changes

webknjaz reviewed Mar 6, 2026

View reviewed changes

KB-perByte and others added 3 commits March 11, 2026 22:58

Link PR ansible#479 to PR ansible#790 change note and review comments

0ebfd42

[pre-commit.ci] auto fixes from pre-commit.com hooks

1acf68f

for more information, see https://pre-commit.ci

Merge branch 'devel' into config_file

7132cc3

fix build and test

7b48c3b

KB-perByte and others added 3 commits March 12, 2026 14:54

check spec option fix

e5905f9

add annotations for 2.9

f887da7

[pre-commit.ci] auto fixes from pre-commit.com hooks

e31a755

for more information, see https://pre-commit.ci

webknjaz reviewed Mar 12, 2026

View reviewed changes

Merge branch 'devel' into config_file

66ef658

Merge branch 'devel' into config_file

00e05c9

KB-perByte and others added 2 commits March 20, 2026 23:45

Getting rid of bytes

d5874e0

[pre-commit.ci] auto fixes from pre-commit.com hooks

4781f5e

for more information, see https://pre-commit.ci

	-- by :user:`KB-perByte`
	-- by :user:`donnerhacke` and :user:`KB-perByte`



		cdef class LibsshConfigParseException(LibsshSessionException):
		"""Raised when SSH config file parsing fails."""

	"""Raised when SSH config file parsing fails."""
	"""A failure to parse or load an SSH configuration file."""

	if path is not None:
	self.parse_config(path)
	self.parse_config(path)

	if path is not None:
	self.parse_config(path)
	self.load_config_into_session(path)

	if path is not None:
	self.parse_config(path)
	self._parse_config(path)

	if path is not None:
	self.parse_config(path)
	self._load_config_into_session(path)

		)


		def test_connect_config_file_none_no_parse():

Uh oh!

Conversation

KB-perByte commented Jan 28, 2026 • edited by webknjaz Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

ISSUE TYPE

ADDITIONAL INFORMATION

Uh oh!

This comment was marked as outdated.

This comment was marked as resolved.

Uh oh!

KB-perByte commented Feb 13, 2026

Uh oh!

Jakuje left a comment

Choose a reason for hiding this comment

Uh oh!

ganeshrn commented Feb 14, 2026

Uh oh!

Jakuje commented Feb 14, 2026

Uh oh!

KB-perByte commented Feb 16, 2026

Uh oh!

Jakuje commented Feb 16, 2026

Uh oh!

KB-perByte commented Feb 16, 2026

Uh oh!

webknjaz commented Feb 16, 2026

Uh oh!

Uh oh!

KB-perByte commented Feb 16, 2026

Uh oh!

webknjaz commented Feb 17, 2026

Uh oh!

webknjaz commented Feb 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webknjaz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webknjaz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webknjaz Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KB-perByte commented Mar 11, 2026

Uh oh!

Jakuje commented Mar 11, 2026

Uh oh!

KB-perByte commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KB-perByte commented Mar 12, 2026

Uh oh!

webknjaz commented Mar 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

KB-perByte commented Jan 28, 2026 •

edited by webknjaz

Loading

webknjaz left a comment •

edited

Loading

webknjaz Mar 6, 2026 •

edited

Loading

KB-perByte commented Mar 12, 2026 •

edited

Loading

codecov Bot commented Mar 20, 2026 •

edited

Loading

KB-perByte commented Mar 20, 2026 •

edited

Loading