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

Handle "Server '' not found in known_hosts exception #1458

Closed
wants to merge 1 commit into from

Conversation

@ldoktor
Copy link

@ldoktor ldoktor commented Apr 22, 2016

The message with "reject_unknown_hosts" and "abort_on_prompts" is rather
missleading as paramiko returns:

SSHException: Server 'localhost' not found in known_hosts

but fabric reports:

Needed to prompt for a connection or sudo password (host:
localhost), but abort-on-prompts was set to True

This patch handles the special case similarly to existing workarounds.

Signed-off-by: Lukáš Doktor ldoktor@redhat.com

@ldoktor
Copy link
Author

@ldoktor ldoktor commented May 9, 2016

Hello @bitprophet, would you please take a look at this simple modification? It improves the error message when "reject_unknown_hosts" and "abort_on_prompts" are used.

@ldoktor
Copy link
Author

@ldoktor ldoktor commented Jun 2, 2016

Hello @bitprophet, might I ask you to take a look at this few-line fix? It shouldn't take long and it significantly improves the possibility to detect known_hosts issues.

@ldoktor
Copy link
Author

@ldoktor ldoktor commented Jun 21, 2016

@bitprophet would you please take a look at this? It's a simple 6-liner.

@bitprophet bitprophet added this to the 1.10.4 milestone Jun 21, 2016
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jun 21, 2016

Thanks, assigning to next bugfix release. If you have time, a changelog entry and tests would be great (though I don't recall if we test the other spots like this one; if we don't, OK to omit.)

@ldoktor ldoktor force-pushed the ldoktor:exc-known_hosts branch 2 times, most recently from 9b5d04f to f58e0dd Jun 22, 2016
@ldoktor
Copy link
Author

@ldoktor ldoktor commented Jun 22, 2016

Thank you, @bitprophet, I added the selftest (not sure it uses all the neat features of fabric selftests, but works fine), but I'm fighting with the changelog, it keeps failing. Do you have any suggestions, please?

The message with "reject_unknown_hosts" and "abort_on_prompts" is rather
missleading as paramiko returns:

    SSHException: Server 'localhost' not found in known_hosts

but fabric reports:

    Needed to prompt for a connection or sudo password (host:
    localhost), but abort-on-prompts was set to True

This patch handles the special case similarly to existing workarounds.

Signed-off-by: Lukáš Doktor <ldoktor@redhat.com>
@ldoktor ldoktor force-pushed the ldoktor:exc-known_hosts branch from f58e0dd to ac8e91b Jun 28, 2016
@ldoktor
Copy link
Author

@ldoktor ldoktor commented Jun 28, 2016

@bitprophet, changelog and selftest added.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Jul 3, 2016

Thanks, will get to this on my next bugfix-merge pass!

jeblundell added a commit to jeblundell/fabric that referenced this pull request Jul 17, 2016
jeblundell added a commit to jeblundell/fabric that referenced this pull request Jul 17, 2016
@bitprophet
Copy link
Member

@bitprophet bitprophet commented Aug 22, 2016

Sanity checked the test and it's a false positive! :( With the fix commented out, test still passes.

Seems most directly due to fact that when password is non-empty (which is the default for the test environment), the block just above the fix fires, still raising a NetworkError, thus still satisfying the test. When I add password=None to the test's settings() call, the test correctly fails (when the patch is commented out).

This then lead me to realize that the fix should also just get bundled up with that earlier NetworkError block, so I have reformatting things that way. Now easier to read, still works the same way, and the updated test now correctly passes-or-fails as appropriate.

I also tested manually with various combos of reject_known_hosts and abort_on_prompts, still seems good.

@bitprophet
Copy link
Member

@bitprophet bitprophet commented Aug 22, 2016

Cherry-picked back to 1.10 & layered on the above fixes, all set now! Thanks!!

@bitprophet bitprophet closed this Aug 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants