CrowdStrike / ansible_collection_falcon

Comprehensive toolkit for streamlining your interactions with the CrowdStrike Falcon platform.
https://galaxy.ansible.com/ui/repo/published/crowdstrike/falcon/
GNU General Public License v3.0
97 stars 60 forks source link

Support for hostname templating #474

Closed Gianlu closed 7 months ago

Gianlu commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

carlosmmatos commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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

Gianlu commented 7 months ago

475

carlosmmatos commented 7 months ago

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.

codecov-commenter commented 7 months ago

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 :warning:

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

Additional details and impacted files ```diff @@ 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](https://app.codecov.io/gh/CrowdStrike/ansible_collection_falcon/pull/474/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [sanity](https://app.codecov.io/gh/CrowdStrike/ansible_collection_falcon/pull/474/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `37.95% <17.64%> (+0.16%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.