Add 'type: string' to connection options to make sure they are actually strings #549
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## main #549 +/- ##
=======================================
Coverage 64.47% 64.47%
=======================================
Files 31 31
Lines 3868 3868
Branches 685 685
=======================================
Hits 2494 2494
Misses 1231 1231
Partials 143 143 |
|
The ansible-lint error is unrelated, it also applies to the |
This was referenced Jun 11, 2023
for more information, see https://pre-commit.ci
Qalthos
approved these changes
Jun 14, 2023
|
@Qalthos thanks for reviewing and merging! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
As opposed to modules, the plugin config system uses
type: rawas a default fortype, and nottype: str. Therefore options that do not specifytypecan end up as non-strings.This caused a problem in ansible-collections/community.routeros#175 when users had passwords that could be interpreted as integers. Specifying
ansible_ssh_pass=1234567890in the inventory caused strange errors in the bowels of network_cli (i.e. without stack traces since the errors happened on the other side of the JSON RPC connection). Once @BSMaximN figured out this is caused by passwords that look like integers, I was able to reproduce this with network_cli via libssh. (I have some trouble getting paramiko to work so I cannot reproduce it there.)The problem seems to boil down to Ansible's plugin config manager having a different default for
typethan the module argument spec: if you omittype, then the values are kept as-is (for modules, they are converted to strings).Therefore the password ended up being an integer and not a string, which trips up both paramiko and libssh.
I've added
type: stringto all options which do not have other types for the connection plugins; addingtype: stringto libssh'spasswordfield solved the problem for me, and I guess there are many similar problems where other values are assumed to be strings that potentially aren't. (There are also a lot ofto_native()s everywhere, so some of them got converted to strings already before.)ISSUE TYPE
COMPONENT NAME
connection plugins