From 37bcd32a972a703133c3fc609165da84e5ab66a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A9ri=20Le=20Bouder?= Date: Thu, 15 Sep 2022 17:45:47 -0400 Subject: [PATCH] inventory/aws_ec2: allow multi-entries for one host Add an option to allow multiple duplicated entry for on single instance. Closes: #950 --- .../fragments/inventory-multi-hosts.yaml | 5 + plugins/inventory/aws_ec2.py | 66 ++++++++-- ...ng_inventory_with_hostnames_using_tags.yml | 6 +- ...tory_with_hostnames_using_tags_classic.yml | 6 +- tests/unit/plugins/inventory/test_aws_ec2.py | 117 ++++++++++++++++-- 5 files changed, 178 insertions(+), 22 deletions(-) create mode 100644 changelogs/fragments/inventory-multi-hosts.yaml diff --git a/changelogs/fragments/inventory-multi-hosts.yaml b/changelogs/fragments/inventory-multi-hosts.yaml new file mode 100644 index 00000000000..7999a7e8ba1 --- /dev/null +++ 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/plugins/inventory/aws_ec2.py b/plugins/inventory/aws_ec2.py index 927014f1c76..1571477460b 100644 --- a/plugins/inventory/aws_ec2.py +++ b/plugins/inventory/aws_ec2.py @@ -64,6 +64,13 @@ type: str default: '_' required: False + allow_duplicated_hosts: + description: + - By default, only the first name that back 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 filters: description: - A dictionary of filter value pairs. @@ -176,6 +183,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 +636,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 +645,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 +731,27 @@ 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 ''' for host in hosts: - hostname_list = self._get_hostname(host, hostnames) + if allow_duplicated_hosts: + hostname_list = self.get_all_hostnames(host, hostnames) + else: + hostname_list = [self._get_preferred_hostname(host, hostnames)] host = camel_dict_to_snake_dict(host, ignore_list=['Tags']) host['tags'] = boto3_tag_list_to_ansible_dict(host.get('tags', [])) @@ -820,6 +869,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 +889,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/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml index 58dff9b8fb6..dfae16f05d0 100644 --- a/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml +++ b/tests/integration/targets/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/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml index 639acca007e..576b53ab549 100644 --- a/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml +++ b/tests/integration/targets/inventory_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 index fcff7ff75ec..c8a44eb5b12 100644 --- 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")