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

Support for hostname templating #474

Merged
merged 5 commits into from
Mar 25, 2024

Conversation

Gianlu
Copy link
Contributor

@Gianlu Gianlu commented Mar 20, 2024

Hi,
I suggest this imporovement in order to be able to do some "post-processing" in inventory name.
In my use case, the vm names are uppercase and the inventory_hostname variable for ansible inventory entries have to be lowercase.
This can be accomplished by:

hostnames:
  - hostname | lower

The actual logic can be achieved by (I don't modified that part)

hostnames:
  - hostname
  - ip_address

Let me know,
Bye

@carlosmmatos
Copy link
Contributor

Hey @Gianlu - I'm not following your use-case entirely. For example, you can already do this in native Ansible/Jinja in a playbook:

- hosts: "{{ target_host | lower }}"
  tasks:
    - name: Example task
      debug:
        msg: "This task runs on host {{ target_host }}"

In other words.. post processing should either be done using the compose function or in a playbook.

Here is another example that may work for you:

Let's say we add this to our dynamic inventory file:

compose:
  ansible_inventory_hostname: "{{ hostname | lower }}"

Then in a playbook, you could do something like this:

- hosts: all
  tasks:
    - name: Example task
      debug:
        msg: "This task runs on host {{ ansible_inventory_hostname }}"

Hopefully this resonates with your end goal.

@Gianlu
Copy link
Contributor Author

Gianlu commented Mar 21, 2024

Hello,
I tried with inventory_hostname but it ditn't work. I'll try with your suggestion but my use case in a bit different.
Long story short: We are not using the inventory to run playbooks but to build a more complex inventory.
We are trying to build an inventory from multiple sources: for our use case, the inventory isn't a file but a directory:

$ tree inventory
inventory
├── 00_virtual_manager01.yaml
├── 00_virtual_manager02.yaml
├── 00_virtual_manager03.yaml
├── 01_our_monitoring.yml
├── 10_cloud.yml
├── 11_our.falcon_hosts.yml
├── 15_ad1.microsoft.ad.ldap.yml
├── 15_ad2.microsoft.ad.ldap.yml
├── 79_groups_vars.yml
└── 80_final.yml

and it's used as:

$ ansible-playbook -i path/to/inventory/directory my_playbook.yml

All files are dynamic inventories. The falcon hosts inventory in the final piece of the puzzle. In order to have the dictionary merged I've two requirements

  1. the inventory hostname have to match cross plugin when the inventory method self.inventory.add_host(hostname) is called.
  2. the Jinja2 templating support in inventory paramenters eg
plugin: crowdstrike.falcon.falcon_hosts
client_id: "{{ lookup('community.hashi_vault.hashi_vault', 'secret=path/to/secret:client_id') }}"
client_secret: "{{ lookup('community.hashi_vault.hashi_vault', 'secret=path/to/secret:client_secret') }}"
cloud: 'eu-1'
filter: "product_type_desc:'Server' + last_seen:>='now-30d' + chassis_type:!'10'"

The first requirement is the scope of this PR.
The last requirement is necessary in order to not go mad with enviroment variables and for using twice the same plugin with different parameters. To achieve that, I patched falcon_hosts.py:

  creds = {}
  for key, env in cred_mapping.items():
      value = self.get_option(key) or os.getenv(env)
      if self.templar.is_template(value):
          value = self.templar.template(variable=value,disable_lookups=False)
      if value:
          if key == "cloud":
              creds["base_url"] = value
          else:
              creds[key] = value

In order to support Hashicoprp Vault lookup or generic lookup.

I hope I was able to explain myself,

Thanks

@carlosmmatos
Copy link
Contributor

Let me think about it for a little bit to see what we can do. Question for you, are there other dynamic inventory plugin's that you use where this feature is already there?

@Gianlu
Copy link
Contributor Author

Gianlu commented Mar 21, 2024

Hi:
I found the hostnames parameters in these inventory plugins:

@carlosmmatos
Copy link
Contributor

carlosmmatos commented Mar 21, 2024

Got you.. thanks for that.. Let me review the PR and make sure there is nothing else we need to add for this. Afterwards, I can start a new PR for the 2nd part of your problem to support that as well, as that is a valid request as well! Thanks again for using this inventory and providing feedback! Hang tight.

@Gianlu
Copy link
Contributor Author

Gianlu commented Mar 21, 2024

If you preferer, for the sesond part, I've already ready the patch in my branch so I can open the PR for your review.

@carlosmmatos
Copy link
Contributor

Yeah go ahead and open the PR.. I'll have some updates for it but it's a great starting point!

@Gianlu
Copy link
Contributor Author

Gianlu commented Mar 21, 2024

#475

@carlosmmatos
Copy link
Contributor

I haven't forgotten about this. In fact, I've been diving a little more into this trying to get a better grasp of error handling and ensuring we get our desired outputs for an inventory. I went ahead and started using the _get_hostname() function that most of the other dynamic inventory files use because it makes more sense, especially adding the strict option to fail.

What I'm noticing though, is that there are things such as automatic deduplication that occurs when trying to use a value such as hostname as the inventory_hostname. This is because there is a possibility that either a) there is a duplicate hostname and b) no hostname exists. What I'm noticing is that a lot of the other dynamic inventories specify that hostname should be a unique value. From Falcon Hosts, one thing unique to each hosts is the AID (or device_id in this case). If you use device_id as the hostname:

hostnames:
  - device_id

You should get a 1:1 return from the API response. The caveat here would be that we would need to set the ansible_host value to be one of['hostname', 'external_ip', 'local_ip'] assuming they exist. Otherwise you wouldn't be able to connect to the host. So this is why it's taking me longer reviewing this PR. I'll go ahead and push my current changes so you can get an idea of where I'm headed with it.

@carlosmmatos carlosmmatos added bugfixes Bug fixes or minor enhancements enhancement New feature or request inventory labels Mar 22, 2024
@carlosmmatos carlosmmatos self-assigned this Mar 22, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2024

Codecov Report

Attention: Patch coverage is 17.64706% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 37.95%. Comparing base (70d9b23) to head (e92ec73).

Files Patch % Lines
plugins/inventory/falcon_hosts.py 17.64% 14 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #474      +/-   ##
==========================================
+ Coverage   37.78%   37.95%   +0.16%     
==========================================
  Files          17       17              
  Lines         831      830       -1     
  Branches      160      159       -1     
==========================================
+ Hits          314      315       +1     
+ Misses        516      514       -2     
  Partials        1        1              
Flag Coverage Δ
sanity 37.95% <17.64%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This helps our docs be clearer as to the potential pitfalls of using hostname
for inventory_hostname due to duplication.
To not upset the Ansible lint gods, these updates provide the same
example set in the 'sample file' format.
redhatrises
redhatrises previously approved these changes Mar 25, 2024
Copy link
Contributor

@carlosmmatos carlosmmatos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ready to merge

@carlosmmatos carlosmmatos merged commit ebf2b33 into CrowdStrike:main Mar 25, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfixes Bug fixes or minor enhancements enhancement New feature or request inventory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants