inventory/aws_ec2: allow multi-entries for one host (ansible-collecti…

…ons#1026) inventory/aws_ec2: allow multi-entries for one host Depends-On: ansible-collections#1066 Add an option to allow multiple duplicated entries for one single instance. Closes: ansible-collections#950 Reviewed-by: Alina Buzachis <None> Reviewed-by: Jill R <None> Reviewed-by: Gonéri Le Bouder <[email protected]> Reviewed-by: Markus Bergholz <[email protected]>
abikouo · Sep 29, 2022 · ac3ccf5 · ac3ccf5
1 parent e929cf6
commit ac3ccf5
Show file tree

Hide file tree

Showing 6 changed files with 186 additions and 23 deletions.
diff --git a/changelogs/fragments/inventory-multi-hosts.yaml b/changelogs/fragments/inventory-multi-hosts.yaml
@@ -0,0 +1,5 @@
+---
+minor_changes:
+- aws_ec2 - introduce the ``allow_duplicated_hosts`` configuration key (https://github.com/ansible-collections/amazon.aws/pull/1026).
+bugfixes:
+- aws_ec2 - address a regression introduced in 4.1.0 (https://github.com/ansible-collections/amazon.aws/pull/862) that cause the presnse of duplicated hosts in the inventory.
diff --git a/docs/docsite/rst/aws_ec2_guide.rst b/docs/docsite/rst/aws_ec2_guide.rst
@@ -158,7 +158,10 @@ Some examples are shown below:
     - dns-name
     - tag:Name
     - private-ip-address
-  
+
+By default, the inventory will only return the first match one of the ``hostnames`` entries.
+You may want to get all the potential matches in your inventory, this also implies you will get
+duplicated entries. To switch to this behavior, set the ``allow_duplicated_hosts`` configuration key to ``True``.
 
 ``keyed_groups``
 ----------------

diff --git a/plugins/inventory/aws_ec2.py b/plugins/inventory/aws_ec2.py
@@ -64,6 +64,14 @@
         type: str
         default: '_'
         required: False
+  allow_duplicated_hosts:
+    description:
+      - By default, the first name that matches an entry of the I(hostnames) list is returned.
+      - Turn this flag on if you don't mind having duplicated entries in the inventory
+        and you want to get all the hostnames that match.
+    type: bool
+    default: False
+    version_added: 5.0.0
   filters:
     description:
       - A dictionary of filter value pairs.
@@ -176,6 +184,9 @@
     separator: '-'       # Hostname will be aws-test_literal
     prefix: 'aws'
 
+# Returns all the hostnames for a given instance
+allow_duplicated_hosts: False
+
 # Example using constructed features to create groups and set ansible_host
 plugin: aws_ec2
 regions:
@@ -626,7 +637,7 @@ def _sanitize_hostname(self, hostname):
         else:
             return to_text(hostname)
 
-    def _get_hostname(self, instance, hostnames):
+    def _get_preferred_hostname(self, instance, hostnames):
         '''
             :param instance: an instance dict returned by boto3 ec2 describe_instances()
             :param hostnames: a list of hostname destination variables in order of preference
@@ -635,14 +646,46 @@ def _get_hostname(self, instance, hostnames):
         if not hostnames:
             hostnames = ['dns-name', 'private-dns-name']
 
+        hostname = None
+        for preference in hostnames:
+            if isinstance(preference, dict):
+                if 'name' not in preference:
+                    raise AnsibleError("A 'name' key must be defined in a hostnames dictionary.")
+                hostname = self._get_preferred_hostname(instance, [preference["name"]])
+                hostname_from_prefix = self._get_preferred_hostname(instance, [preference["prefix"]])
+                separator = preference.get("separator", "_")
+                if hostname and hostname_from_prefix and 'prefix' in preference:
+                    hostname = hostname_from_prefix + separator + hostname
+            elif preference.startswith('tag:'):
+                tags = self._get_tag_hostname(preference, instance)
+                hostname = tags[0] if tags else None
+            else:
+                hostname = self._get_boto_attr_chain(preference, instance)
+            if hostname:
+                break
+        if hostname:
+            if ':' in to_text(hostname):
+                return self._sanitize_group_name((to_text(hostname)))
+            else:
+                return to_text(hostname)
+
+    def get_all_hostnames(self, instance, hostnames):
+        '''
+            :param instance: an instance dict returned by boto3 ec2 describe_instances()
+            :param hostnames: a list of hostname destination variables
+            :return all the candidats matching the expectation
+        '''
+        if not hostnames:
+            hostnames = ['dns-name', 'private-dns-name']
+
         hostname = None
         hostname_list = []
         for preference in hostnames:
             if isinstance(preference, dict):
                 if 'name' not in preference:
                     raise AnsibleError("A 'name' key must be defined in a hostnames dictionary.")
-                hostname = self._get_hostname(instance, [preference["name"]])
-                hostname_from_prefix = self._get_hostname(instance, [preference["prefix"]])
+                hostname = self.get_all_hostnames(instance, [preference["name"]])
+                hostname_from_prefix = self.get_all_hostnames(instance, [preference["prefix"]])
                 separator = preference.get("separator", "_")
                 if hostname and hostname_from_prefix and 'prefix' in preference:
                     hostname = hostname_from_prefix[0] + separator + hostname[0]
@@ -689,20 +732,30 @@ def _query(self, regions, include_filters, exclude_filters, strict_permissions):
 
         return {'aws_ec2': instances}
 
-    def _populate(self, groups, hostnames):
+    def _populate(self, groups, hostnames, allow_duplicated_hosts=False):
         for group in groups:
             group = self.inventory.add_group(group)
-            self._add_hosts(hosts=groups[group], group=group, hostnames=hostnames)
+            self._add_hosts(
+                hosts=groups[group],
+                group=group,
+                hostnames=hostnames,
+                allow_duplicated_hosts=allow_duplicated_hosts)
             self.inventory.add_child('all', group)
 
-    def _add_hosts(self, hosts, group, hostnames):
+    def _add_hosts(self, hosts, group, hostnames, allow_duplicated_hosts=False):
         '''
             :param hosts: a list of hosts to be added to a group
             :param group: the name of the group to which the hosts belong
             :param hostnames: a list of hostname destination variables in order of preference
+            :param bool allow_duplicated_hosts: allow multiple copy for the same host with a different name
         '''
         for host in hosts:
-            hostname_list = self._get_hostname(host, hostnames)
+            if allow_duplicated_hosts:
+                hostname_list = self.get_all_hostnames(host, hostnames)
+            elif preferred_hostname := self._get_preferred_hostname(host, hostnames):
+                hostname_list = [preferred_hostname]
+            else:
+                continue
 
             host = camel_dict_to_snake_dict(host, ignore_list=['Tags'])
             host['tags'] = boto3_tag_list_to_ansible_dict(host.get('tags', []))
@@ -820,6 +873,7 @@ def parse(self, inventory, loader, path, cache=True):
         exclude_filters = self.get_option('exclude_filters')
         hostnames = self.get_option('hostnames')
         strict_permissions = self.get_option('strict_permissions')
+        allow_duplicated_hosts = self.get_option('allow_duplicated_hosts')
 
         cache_key = self.get_cache_key(path)
         # false when refresh_cache or --flush-cache is used
@@ -839,7 +893,7 @@ def parse(self, inventory, loader, path, cache=True):
         if not cache or cache_needs_update:
             results = self._query(regions, include_filters, exclude_filters, strict_permissions)
 
-        self._populate(results, hostnames)
+        self._populate(results, hostnames, allow_duplicated_hosts=allow_duplicated_hosts)
 
         # If the cache has expired/doesn't exist or if refresh_inventory/flush cache is used
         # when the user is using caching, update the cached inventory

diff --git a/...rgets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml b/...rgets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml
@@ -40,11 +40,11 @@
           assert:
             that:
               - "'aws_ec2' in groups"
-              - groups['aws_ec2'] | length == 2
+              - groups['aws_ec2'] | length == 1
               - "'Tag1_Test1' in groups['aws_ec2']"
-              - "'Tag2_Test2' in groups['aws_ec2']"
+              - "'Tag2_Test2' not in groups['aws_ec2']"
               - "'Tag1_Test1' in hostvars"
-              - "'Tag2_Test2' in hostvars"
+              - "'Tag2_Test2' not in hostvars"
 
       always:
 

diff --git a/...ventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml b/...ventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml
@@ -40,11 +40,11 @@
           assert:
             that:
               - "'aws_ec2' in groups"
-              - groups['aws_ec2'] | length == 2
+              - groups['aws_ec2'] | length == 1
               - "'Test1' in groups['aws_ec2']"
-              - "'Test2' in groups['aws_ec2']"
+              - "'Test2' not in groups['aws_ec2']"
               - "'Test1' in hostvars"
-              - "'Test2' in hostvars"
+              - "'Test2' not in hostvars"
 
       always:
 

diff --git a/tests/unit/plugins/inventory/test_aws_ec2.py b/tests/unit/plugins/inventory/test_aws_ec2.py
@@ -23,6 +23,7 @@
 
 import pytest
 import datetime
+from unittest.mock import MagicMock
 
 from ansible.errors import AnsibleError
 from ansible.parsing.dataloader import DataLoader
@@ -108,9 +109,25 @@
 }
 
 
-@pytest.fixture(scope="module")
+@pytest.fixture()
 def inventory():
-    return InventoryModule()
+    inventory = InventoryModule()
+    inventory._options = {
+        "aws_profile": "first_precedence",
+        "aws_access_key": "test_access_key",
+        "aws_secret_key": "test_secret_key",
+        "aws_security_token": "test_security_token",
+        "iam_role_arn": None,
+        "use_contrib_script_compatible_ec2_tag_keys": False,
+        "hostvars_prefix": "",
+        "hostvars_suffix": "",
+        "strict": True,
+        "compose": {},
+        "groups": {},
+        "keyed_groups": [],
+    }
+    inventory.inventory = MagicMock()
+    return inventory
 
 
 def test_compile_values(inventory):
@@ -139,21 +156,50 @@ def test_boto3_conn(inventory):
             assert "Insufficient credentials found" in error_message
 
 
-def test_get_hostname_default(inventory):
+def testget_all_hostnames_default(inventory):
     instance = instances['Instances'][0]
-    assert inventory._get_hostname(instance, hostnames=None)[0] == "ec2-12-345-67-890.compute-1.amazonaws.com"
+    assert inventory.get_all_hostnames(instance, hostnames=None) == ["ec2-12-345-67-890.compute-1.amazonaws.com", "ip-098-76-54-321.ec2.internal"]
 
 
-def test_get_hostname(inventory):
+def testget_all_hostnames(inventory):
     hostnames = ['ip-address', 'dns-name']
     instance = instances['Instances'][0]
-    assert inventory._get_hostname(instance, hostnames)[0] == "12.345.67.890"
+    assert inventory.get_all_hostnames(instance, hostnames) == ["12.345.67.890", "ec2-12-345-67-890.compute-1.amazonaws.com"]
 
 
-def test_get_hostname_dict(inventory):
+def testget_all_hostnames_dict(inventory):
     hostnames = [{'name': 'private-ip-address', 'separator': '_', 'prefix': 'tag:Name'}]
     instance = instances['Instances'][0]
-    assert inventory._get_hostname(instance, hostnames)[0] == "aws_ec2_098.76.54.321"
+    assert inventory.get_all_hostnames(instance, hostnames) == ["aws_ec2_098.76.54.321"]
+
+
+def testget_all_hostnames_with_2_tags(inventory):
+    hostnames = ['tag:ansible', 'tag:Name']
+    instance = instances['Instances'][0]
+    assert inventory.get_all_hostnames(instance, hostnames) == ["test", "aws_ec2"]
+
+
+def test_get_preferred_hostname_default(inventory):
+    instance = instances['Instances'][0]
+    assert inventory._get_preferred_hostname(instance, hostnames=None) == "ec2-12-345-67-890.compute-1.amazonaws.com"
+
+
+def test_get_preferred_hostname(inventory):
+    hostnames = ['ip-address', 'dns-name']
+    instance = instances['Instances'][0]
+    assert inventory._get_preferred_hostname(instance, hostnames) == "12.345.67.890"
+
+
+def test_get_preferred_hostname_dict(inventory):
+    hostnames = [{'name': 'private-ip-address', 'separator': '_', 'prefix': 'tag:Name'}]
+    instance = instances['Instances'][0]
+    assert inventory._get_preferred_hostname(instance, hostnames) == "aws_ec2_098.76.54.321"
+
+
+def test_get_preferred_hostname_with_2_tags(inventory):
+    hostnames = ['tag:ansible', 'tag:Name']
+    instance = instances['Instances'][0]
+    assert inventory._get_preferred_hostname(instance, hostnames) == "test"
 
 
 def test_set_credentials(inventory):
@@ -216,3 +262,58 @@ def test_include_filters_with_filter_and_include_filters(inventory):
     assert inventory.build_include_filters() == [
         {"from_filter": 1},
         {"from_include_filter": "bar"}]
+
+
+def test_add_host_empty_hostnames(inventory):
+    hosts = [
+        {
+            "Placement": {
+                "AvailabilityZone": "us-east-1a",
+            },
+            "PublicDnsName": "ip-10-85-0-4.ec2.internal"
+        },
+    ]
+    inventory._add_hosts(hosts, "aws_ec2", [])
+    inventory.inventory.add_host.assert_called_with("ip-10-85-0-4.ec2.internal", group="aws_ec2")
+
+
+def test_add_host_with_hostnames_and_one_criteria(inventory):
+    hosts = [{
+        "Placement": {
+            "AvailabilityZone": "us-east-1a",
+        },
+        "PublicDnsName": "sample-host",
+    }]
+
+    inventory._add_hosts(hosts, "aws_ec2", hostnames=["tag:Name", "private-dns-name", "dns-name"])
+    assert inventory.inventory.add_host.call_count == 1
+    inventory.inventory.add_host.assert_called_with("sample-host", group="aws_ec2")
+
+
+def test_add_host_with_hostnames_and_two_matching_criteria(inventory):
+    hosts = [{
+        "Placement": {
+            "AvailabilityZone": "us-east-1a",
+        },
+        "PublicDnsName": "name-from-PublicDnsName",
+        "Tags": [{"Value": "name-from-tag-Name", "Key": "Name"}],
+    }]
+
+    inventory._add_hosts(hosts, "aws_ec2", hostnames=["tag:Name", "private-dns-name", "dns-name"])
+    assert inventory.inventory.add_host.call_count == 1
+    inventory.inventory.add_host.assert_called_with("name-from-tag-Name", group="aws_ec2")
+
+
+def test_add_host_with_hostnames_and_two_matching_criteria_and_allow_duplicated_hosts(inventory):
+    hosts = [{
+        "Placement": {
+            "AvailabilityZone": "us-east-1a",
+        },
+        "PublicDnsName": "name-from-PublicDnsName",
+        "Tags": [{"Value": "name-from-tag-Name", "Key": "Name"}],
+    }]
+
+    inventory._add_hosts(hosts, "aws_ec2", hostnames=["tag:Name", "private-dns-name", "dns-name"], allow_duplicated_hosts=True)
+    assert inventory.inventory.add_host.call_count == 2
+    inventory.inventory.add_host.assert_any_call("name-from-PublicDnsName", group="aws_ec2")
+    inventory.inventory.add_host.assert_any_call("name-from-tag-Name", group="aws_ec2")