debops / debops-tools

Your Debian-based data center in a box
https://debops.org/
GNU General Public License v3.0
1.07k stars 116 forks source link

quoting directories with spaces in ansible.cfg breaks with 2.7 #200

Closed luken closed 6 years ago

luken commented 6 years ago

Symptom

Ansible would not find connection plugins listed in the connection_plugins directive.

fatal: [ans.ap]: FAILED! => { "msg": "the connection plugin 'lxc_ssh' was not found" }

Analysis

Some of the directories in connection_plugins in ansible.cfg had a space, thus triggering this bit of code in debops.

        if ansible.__version__ >= "1.7":
           # work around a bug obviously introduced in 1.7, see
           # https://github.com/ansible/ansible/issues/8555
           if ' ' in defaults[plugin_type]:
               defaults[plugin_type] = PATHSEP.join(
                   '"%s"' % p for p in defaults[plugin_type].split(PATHSEP))

which changes the ansible.cfg lines to look like this

connection_plugins = "/path/to/dir1":"/path/to/dir/2":"/path/to/dir/3 with space/"

rather than this style

connection_plugins = /path/to/dir1:/path/to/dir/2:/path/to/dir/3 with space/

Test case is using ansible 2.7.14 with the quoted directories in the config file the leading/trailing quotation marks get passed down into ansibles guts, eventually causing badness in: ansible/utils/path.py: unfrackpath()

if not os.path.isabs(final_path):

at this point final_path still has the quotation marks:

"/path/to/dir1"

Thus the .isabs() test fails and the path gets munged into an invalid path as seen below that can not be searched

/path/to/basedir/"path/to/dir1"

Fix

The below change fixes the issue on my system using asnbile 2.7.14. According to the ansible ticket linked in the code the ansible issue it was fixed prior to the 2.0 release, so assuming it is the correct version to plugin to the test:

if ansible.__version__ >= "1.7" and ansible.__version__ < "2.0":
luken commented 6 years ago

$ cat /tmp/debops_ansible_version_path_handling.patch

--- debops.orig 2018-01-15 13:05:04.000000000 -0700
+++ debops  2018-01-15 13:24:30.000000000 -0700
@@ -106,7 +106,7 @@
         plugin_type = plugin_type+"_plugins"
         defaults[plugin_type] = PATHSEP.join(custom_paths(plugin_type))

-        if ansible.__version__ >= "1.7":
+        if ansible.__version__ >= "1.7" and ansible.__version__ < "2.0":
             # work around a bug obviously introduced in 1.7, see
             # https://github.com/ansible/ansible/issues/8555
             if ' ' in defaults[plugin_type]:
drybjed commented 6 years ago

Thanks for the bug report! Unfortunately I don't have a macOS host handy to test this... Could you check if your fix works with the version currently present in the debops/debops repository?

If not, that's fine - I can apply the fix and it will probably work. I plan to release a new version with Python 3 support in a few days.

luken commented 6 years ago

Is this good enough of a test?

$ PYTHON_PATH=../debops/lib/debops-tools/debops ; rm ansible.cfg && ../debops/lib/debops-tools/bin/debops -l ans.ap

It still generates the same ansible.cfg with the extra quote characters that ansible chokes on.

drybjed commented 6 years ago

And it works fine with your fix?

luken commented 6 years ago

Testing...

$ patch < /tmp/debops_ansible_version_path_handling.patch patching file debops Hunk #1 succeeded at 119 (offset 13 lines).

With patched version run with same command above.

Also, the python path above was likely not right. Re-ran both cases with PYTHON_PATH=../debops/lib/debops-tools

Same results, unpatched fails, patched works

drybjed commented 6 years ago

@luken Thanks for checking this. In that case I'm gonna apply your patch, it should be fixed in the next release.