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

Cannot pass look_for_keys via FS URL #52

Open
michaelackermannaiub opened this issue Feb 28, 2022 · 3 comments
Open

Cannot pass look_for_keys via FS URL #52

michaelackermannaiub opened this issue Feb 28, 2022 · 3 comments
Labels
enhancement something could be better

Comments

@michaelackermannaiub
Copy link

While it is possible to pass look_for_keys to the underlying paramiko Client as follows

from fs.sshfs import SSHFS

my_fs = SSHFS(..., look_for_keys=False)

I am unable to do so when using an FS URL:

import fs

my_fs = fs.open_fs("ssh://[user[:password]@]host[:port]/[directory]?look-for-keys=False")
my_fs = fs.open_fs("ssh://[user[:password]@]host[:port]/[directory]?look_for_keys=False")

Looking at SSHOpener, this could be remedied by passing look_for_keys in when constructing SSHFS, much like e.g. pkey is.

@althonos
Copy link
Owner

Hi @michaelackermannaiub ,

do you feel like opening a PR for this? I think you could make it work by adding a new keyword argument to the SSHFS constructor (right now, it'll always look for keys unless an argument is given to pkey, this could be changed by another argument) and then updating the opener.

@althonos althonos added the enhancement something could be better label Feb 28, 2022
@michaelackermannaiub
Copy link
Author

Sure!

I can see two ways to implement this:

  1. look_for_keys passed to paramiko.SSHClient.connect no longer depends on the value of pkey passed to SSHFS and is given a sensible default value (I would guess True, in order to retain the current behavior).
  2. look_for_keys passed to paramiko.SSHClient.connect does depend on the value of pkey passed to SSHFS as it does now, but can be overriden by look_for_keys.

I would naively expect the second option, as it retains the current behavior when look_for_keys is not specified.

On a somewhat independent note: While trying to understand pkey, I've come across the following:

The implementation in the SSHFS constructor looks to me like look_for_keys passed to paramiko.SSHClient.connect will always be True:

# Either pkey, keyfile, or both are None
# https://github.com/althonos/fs.sshfs/blob/master/fs/sshfs/sshfs.py#L116
pkey, keyfile = (pkey, None) if isinstance(pkey, paramiko.PKey) else (None, pkey)
...
# look_for_keys will always be True, since (None and x) and (x and None) will always None for all x
# https://github.com/althonos/fs.sshfs/blob/master/fs/sshfs/sshfs.py#L134
look_for_keys = True if (pkey and keyfile) is None else False

This seems at odds with your statement above.

it'll always look for keys unless an argument is given to pkey

@michaelackermannaiub
Copy link
Author

michaelackermannaiub commented Feb 28, 2022

Elaboration on the above:

An example implementation (using the second option from above):
https://github.com/michaelackermannaiub/fs.sshfs/tree/look-for-keys-as-fs-url-parameter

As part of the tests, I tried to understand the interaction between pkey and look_for_keys, and was caught off-guard:
https://github.com/michaelackermannaiub/fs.sshfs/blob/look-for-keys-as-fs-url-parameter/tests/test_url.py#L63
(These tests pass, but I would expect the one with the FIXME to be different based on your statement and the documentation.)

If that's the desired behavior, I'll make this a PR.

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

No branches or pull requests

2 participants