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

parse inventory using Ansible Python API #215

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

quale1
Copy link

@quale1 quale1 commented Apr 21, 2020

Some changes to optionally use the Ansible Python API to parse the inventory files. This could replace the builtin parser which would lead to some simplification, but I took the more conservative approach of leaving the builtin parse intact and adding a --use-ansible-api flag to select this as an alternative parser.

There are a few things that the builtin ansible-cmdb parser doesn't do the same as Ansible does, especially in parsing :var sections and in the order of reading inventory files. (When multiple inventory files are read from a directory including the group_vars and host_vars cases, the files should be read in sorted order. Then there's ansible_group_priority, which I don't use and wouldn't want to implement.)

An earlier experiment with running the ansible-inventory command to parse the inventory revealed that the mixed static and dynamic inventory test has a bug that makes ansible-cmdb parse it differently than ansible does. Ansible requires that every host be in some group, and test/f_inventory/mixeddir/dyninv.py generates json that has a couple hosts that aren't in any group. It looks like Ansible uses a dummy "ungrouped" group in that case, so that's how I fixed the test data. The test of the builtin ansible-cmdb inventory parser passes without modification and this lets parsers based on Ansible code or commands pass as well. (The ansible-inventory experiment was successful, but it seems better to use the Ansible Python API directly rather than spawning another program and parsing its output.)

It isn't easy to import the Ansible libraries without renaming src/ansiblecmdb/ansible.py because a file in the directory named ansible.py will confound the import. I renamed the file to ansible_cmdb.py so that from ansible.parsing.dataloader import DataLoader, etc. work as expected. I also made minimal changes to ansible_cmdb.py (formerly ansible.py) to make it a more useful base class by moving some code out of __init__ to expose key methods to override, specifically load_inventories.

The tests are duplicated because I was too lazy to use dependency injection to specify whether to use Ansible or AnsibleViaInventory.

A random nit: The docstring for the get_hosts method says "Return a list of parsed hosts info, with the limit applied if required", but the method returns a dict of hosts info rather than a list.

@fboender
Copy link
Owner

fboender commented Sep 4, 2021

Apologies for the extremely long delay in responding. I was dealing with some personal issues.

Is PR #214 still relevant given this PR?

@fboender
Copy link
Owner

fboender commented Sep 4, 2021

This could replace the builtin parser which would lead to some simplification

I'd very much like to get rid of the builtin parser. It's been nothing but a headache, but at the time ansible itself wasn't usable as a library. My main concern is that people would have to have ansible itself also installed. Then again, why wouldn't you?

I haven't read and reviewed this PR yet. I hope to be able to do so soon and merge it. It'll require some extensive testing.

@gaelgatelement
Copy link

Testing this PR, it seems that it is not compatible with vaulted files. I'm running the following, where I'm poiting -i to an inventory directory containing vaults :

Traceback (most recent call last):
  File "/home/gaelg/.local/lib/python3.10/site-packages/ansible/plugins/vars/host_group_vars.py", line 109, in get_vars
    new_data = loader.load_from_file(found, cache=True, unsafe=True)
  File "/home/gaelg/.local/lib/python3.10/site-packages/ansible/parsing/dataloader.py", line 94, in load_from_file
    (b_file_data, show_content) = self._get_file_contents(file_name)
  File "/home/gaelg/.local/lib/python3.10/site-packages/ansible/parsing/dataloader.py", line 167, in _get_file_contents
    return self._decrypt_if_vault_data(data, b_file_name)
  File "/home/gaelg/.local/lib/python3.10/site-packages/ansible/parsing/dataloader.py", line 137, in _decrypt_if_vault_data
    b_data = self._vault.decrypt(b_vault_data, filename=b_file_name)
  File "/home/gaelg/.local/lib/python3.10/site-packages/ansible/parsing/vault/__init__.py", line 640, in decrypt
    plaintext, vault_id, vault_secret = self.decrypt_and_get_vault_id(vaulttext, filename=filename, obj=obj)
  File "/home/gaelg/.local/lib/python3.10/site-packages/ansible/parsing/vault/__init__.py", line 657, in decrypt_and_get_vault_id
    raise AnsibleVaultError("A vault password must be specified to decrypt data")
ansible.parsing.vault.AnsibleVaultError: A vault password must be specified to decrypt data

Notice that I exported the ansible vault password in ANSIBLE_VAULT_PASSWORD, but it does not fix the error.

@gaelgatelement
Copy link

gaelgatelement commented Mar 2, 2022

I added support for ansible-vault in this PR. quale1#1

Warning : Rendering this with host details will render all the vaulted values in clear text into the rendered file. This can make secrets leak.

@dirks
Copy link

dirks commented Mar 12, 2023

I encountered a few issues with this pull request:

  1. The --limit flag did not work with --use-ansible-api (no test to catch that yet). For one because self.limit = self._parse_limit(limit) in the Ansible class made the limit value unusable for the InventoryManager. The other issue was that dicts in python 3 are not allowed to change size during iteration.
  2. The custom variables were littered with ansible_* variables, at least when using the --fact-cache flag.
  3. The same but for services variable if the results of service_facts were present.
  4. Nested variables are sometimes not rendered correctly, e.g. Screenshot_20230312_203141
  5. As mentioned by @gaelgatelement vault was not supported
    1. When vault is supported as suggested, vault values can be leaked.
    2. Even with the fix from @gaelgatelement environment variables are only one way to make the vault-pass available.

At master...dirks:ansible-cmdb:vault I rebased the work from @quale1 and @gaelgatelement on current master (3f3e412) and addressed 1, 2, 3 and partly 5.i (if you use the vault_ indirection as suggested in https://docs.ansible.com/ansible/latest/tips_tricks/ansible_tips_tricks.html#keep-vaulted-variables-safely-visible).

I do not know what causes 4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants