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

Add better error messages and checking to known_hosts #38307

Merged
merged 1 commit into from May 17, 2018

Conversation

samdoran
Copy link
Contributor

@samdoran samdoran commented Apr 4, 2018

SUMMARY

Add better error messages to explain why things are failing.

Add integration tests to test some failure scenarios.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

known_hosts.py

ANSIBLE VERSION
2.6

@ansibot
Copy link
Contributor

ansibot commented Apr 4, 2018

cc @mcv21
click here for bot help

@ansibot ansibot added bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests. labels Apr 4, 2018
@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Apr 5, 2018
@mcv21
Copy link

mcv21 commented Apr 6, 2018

Do we really need to specifically handle one particular form of incorrect user input?

@samdoran
Copy link
Contributor Author

samdoran commented Apr 6, 2018

I spent a few hours fighting with this module the other day due to passing in a comma separated list in the name field, so I figured it would be better to explain why it's failing and save others some pain. The key will accept the comma separated list of names, but ssh-keygen only accepts a single name when validating. And it's failure give no indication that that is the problem.

@mcv21
Copy link

mcv21 commented Apr 6, 2018

fair enough!

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 14, 2018
@samdoran
Copy link
Contributor Author

rebuild_merge

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 17, 2018
@ansibot ansibot merged commit 13aff08 into ansible:devel May 17, 2018
samdoran added a commit to samdoran/ansible that referenced this pull request May 21, 2018
samdoran added a commit that referenced this pull request May 22, 2018
achinthagunasekara pushed a commit to achinthagunasekara/ansible that referenced this pull request May 23, 2018
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
@ansible ansible locked and limited conversation to collaborators May 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. support:community This issue/PR relates to code supported by the Ansible community. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants