Make SCP and StartShell behaves the same way as Device for SSH - #649
Make SCP and StartShell behaves the same way as Device for SSH#649vincentbernat wants to merge 4 commits into
Conversation
This is a followup of Juniper#628 where this change was initially pushed but reverted because I didn't remember why I did it. Before Juniper#628, when an identity is provided in the SSH configuration, it was not copied in `_conf_ssh_private_key_file` due to a bug. After fixing the bug in Juniper#628, the key is now copied. However, the SSH configuration is provided to the `connect()` method which will use it if needed. Therefore, this is not needed. Moreover, if the key is provided by an agent and/or encrypted, this won't work as, later in the code, `allow_agent` will be set to `False` due to the presence of a private key.
The `allow_agent` setting was used with NC but not with SCP. Store it in the device and reuse it for SCP. The private key was stored in a dictionnary, but this is not needed as Paramiko's `connect()` would default to `None` when not provided. Grab the SSH key filename from SSH configuration as Paramiko won't do it for us. For this reason, this commit is a followup to the one in Juniper#648.
This is a preparation to reuse the client for other purposes, notably for starting a shell.
|
Autobot: Would an admin like to run functional tests? |
|
Autobot: Would an admin like to run functional tests? |
|
ok to test. |
| # we want to enable the ssh-agent if-and-only-if we are | ||
| # not given a password or an ssh key file. | ||
| # in this condition it means we want to query the agent | ||
| # for available ssh keys |
There was a problem hiding this comment.
But we can also have ssh key file protected by passphrase. That passphrase in passed to Device as password.
There was a problem hiding this comment.
If you provide a password, the agent is disabled and Paramiko will fetch the key and decrypt it. The agent doesn't allow you to provide a password for an encrypted key either (it will ask the user for that if needed). Moreover, this code is already present as is. It's just moved here to be able to reuse its result for scp.
IMO, the ability to disable the agent part is odd. The regular "ssh" client doesn't have such an option. It adds complexity and I don't know why this has been implemented. We should just get rid of that.
|
Can one of the admins verify this patch? |
|
ok to test |
|
@stacywsmith Can you please review this pull request. |
|
Build finished. 551 tests run, 0 skipped, 0 failed. |
4 participants
This is a followup to #648 (and this PR includes the commit from #648, so it shouldn't be merged before #648).
First commit makes small changes to SCP. This is mostly cosmetic with the exception of the agent and the private key. This is quite similar to what ncclient is doing with the same parameters that were provided in
Device.The second commit moves the creation of the paramiko SSH client into a dedicated function.
The third commit reuses this dedicated function for
StartShell().