From ca33389025293863a0884a0a0295bb4cfd46688c Mon Sep 17 00:00:00 2001 From: Alina Buzachis Date: Fri, 24 Jun 2022 12:39:10 +0200 Subject: [PATCH] aws_ec2 fix hostnames (#862) aws_ec2 fix hostnames SUMMARY aws_ec2- fix hostnames bugs Should fix #582 and #583 Fix returned hosts when hostnames: - tag:Tag1,Tag2 Fix returned hosts when hostnames: - tag:Tag1 - tag:Tag2 Fix returned hosts when hostnames: - tag:Tag1=Test1,Tag2=Test2 Given and EC2 instance with the following tags tags: Tag1: Test1 Tag2: Test2 Instead of only returning "aws_ec2": { "hosts": [ "Test1" ] }, "tag_Name_instance_02": { "hosts": [ "Test1" ] }, "tag_Tag1_Test1": { "hosts": [ "Test1" ] }, "tag_Tag2_Test2": { "hosts": [ "Test1" ] } It returns now { "_meta": { "hostvars": { "Test1": { "ami_launch_index": 0, "ansible_host": "10.210.0.101", ...}, "Test2": { "ami_launch_index": 0, "ansible_host": "10.210.0.101", ...}, }, "all": { "children": [ "aws_ec2", "tag_Name_instance_02", "tag_Tag1_Test1", "tag_Tag2_Test2", "ungrouped" ] }, "aws_ec2": { "hosts": [ "Test1", "Test2" ] }, "tag_Name_instance_02": { "hosts": [ "Test1", "Test2" ] }, "tag_Tag1_Test1": { "hosts": [ "Test1", "Test2" ] }, "tag_Tag2_Test2": { "hosts": [ "Test1", "Test2" ] } } ISSUE TYPE Bugfix Pull Request COMPONENT NAME aws_ec2 ADDITIONAL INFORMATION Reviewed-by: Mark Chappell Reviewed-by: Alina Buzachis Reviewed-by: Markus Bergholz --- .../fragments/862-aws_ec2-hostnames.yml | 2 + plugins/inventory/aws_ec2.py | 52 ++++++++++------ ...ng_inventory_with_hostnames_using_tags.yml | 62 +++++++++++++++++++ ...tory_with_hostnames_using_tags_classic.yml | 62 +++++++++++++++++++ .../targets/inventory_aws_ec2/runme.sh | 4 ++ ...inventory_with_hostnames_using_tags.yml.j2 | 21 +++++++ ...y_with_hostnames_using_tags_classic.yml.j2 | 21 +++++++ tests/unit/plugins/inventory/test_aws_ec2.py | 6 +- 8 files changed, 208 insertions(+), 22 deletions(-) create mode 100644 changelogs/fragments/862-aws_ec2-hostnames.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags.yml.j2 create mode 100644 tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags_classic.yml.j2 diff --git a/changelogs/fragments/862-aws_ec2-hostnames.yml b/changelogs/fragments/862-aws_ec2-hostnames.yml new file mode 100644 index 00000000000..5202e488b24 --- /dev/null +++ b/changelogs/fragments/862-aws_ec2-hostnames.yml @@ -0,0 +1,2 @@ +bugfixes: +- aws_ec2 - ensure the correct number of hosts are returned when tags as hostnames are used (https://github.com/ansible-collections/amazon.aws/pull/862). diff --git a/plugins/inventory/aws_ec2.py b/plugins/inventory/aws_ec2.py index 14bd70e54af..6f3d1d5ef14 100644 --- a/plugins/inventory/aws_ec2.py +++ b/plugins/inventory/aws_ec2.py @@ -606,17 +606,25 @@ def _get_tag_hostname(self, preference, instance): tag_hostnames = tag_hostnames.split(',') else: tag_hostnames = [tag_hostnames] + tags = boto3_tag_list_to_ansible_dict(instance.get('Tags', [])) + tag_values = [] for v in tag_hostnames: if '=' in v: tag_name, tag_value = v.split('=') if tags.get(tag_name) == tag_value: - return to_text(tag_name) + "_" + to_text(tag_value) + tag_values.append(to_text(tag_name) + "_" + to_text(tag_value)) else: tag_value = tags.get(v) if tag_value: - return to_text(tag_value) - return None + tag_values.append(to_text(tag_value)) + return tag_values + + def _sanitize_hostname(self, hostname): + if ':' in to_text(hostname): + return self._sanitize_group_name(to_text(hostname)) + else: + return to_text(hostname) def _get_hostname(self, instance, hostnames): ''' @@ -628,6 +636,7 @@ def _get_hostname(self, instance, hostnames): hostnames = ['dns-name', 'private-dns-name'] hostname = None + hostname_list = [] for preference in hostnames: if isinstance(preference, dict): if 'name' not in preference: @@ -636,18 +645,20 @@ def _get_hostname(self, instance, hostnames): hostname_from_prefix = self._get_hostname(instance, [preference["prefix"]]) separator = preference.get("separator", "_") if hostname and hostname_from_prefix and 'prefix' in preference: - hostname = hostname_from_prefix + separator + hostname + hostname = hostname_from_prefix[0] + separator + hostname[0] elif preference.startswith('tag:'): hostname = self._get_tag_hostname(preference, instance) 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) + if isinstance(hostname, list): + for host in hostname: + hostname_list.append(self._sanitize_hostname(host)) + elif isinstance(hostname, str): + hostname_list.append(self._sanitize_hostname(hostname)) + + return hostname_list def _query(self, regions, include_filters, exclude_filters, strict_permissions): ''' @@ -691,7 +702,7 @@ def _add_hosts(self, hosts, group, hostnames): :param hostnames: a list of hostname destination variables in order of preference ''' for host in hosts: - hostname = self._get_hostname(host, hostnames) + hostname_list = self._get_hostname(host, hostnames) host = camel_dict_to_snake_dict(host, ignore_list=['Tags']) host['tags'] = boto3_tag_list_to_ansible_dict(host.get('tags', [])) @@ -703,9 +714,10 @@ def _add_hosts(self, hosts, group, hostnames): # Allow easier grouping by region host['placement']['region'] = host['placement']['availability_zone'][:-1] - if not hostname: + if not hostname_list: continue - self.inventory.add_host(hostname, group=group) + for hostname in hostname_list: + self.inventory.add_host(to_text(hostname), group=group) hostvars_prefix = self.get_option("hostvars_prefix") hostvars_suffix = self.get_option("hostvars_suffix") new_vars = dict() @@ -715,7 +727,8 @@ def _add_hosts(self, hosts, group, hostnames): if hostvars_suffix: hostvar = hostvar + hostvars_suffix new_vars[hostvar] = hostval - self.inventory.set_variable(hostname, hostvar, hostval) + for hostname in hostname_list: + self.inventory.set_variable(to_text(hostname), hostvar, hostval) host.update(new_vars) # Use constructed if applicable @@ -723,13 +736,14 @@ def _add_hosts(self, hosts, group, hostnames): strict = self.get_option('strict') # Composed variables - self._set_composite_vars(self.get_option('compose'), host, hostname, strict=strict) + for hostname in hostname_list: + self._set_composite_vars(self.get_option('compose'), host, to_text(hostname), strict=strict) - # Complex groups based on jinja2 conditionals, hosts that meet the conditional are added to group - self._add_host_to_composed_groups(self.get_option('groups'), host, hostname, strict=strict) + # Complex groups based on jinja2 conditionals, hosts that meet the conditional are added to group + self._add_host_to_composed_groups(self.get_option('groups'), host, to_text(hostname), strict=strict) - # Create groups based on variable values and add the corresponding hosts to it - self._add_host_to_keyed_groups(self.get_option('keyed_groups'), host, hostname, strict=strict) + # Create groups based on variable values and add the corresponding hosts to it + self._add_host_to_keyed_groups(self.get_option('keyed_groups'), host, to_text(hostname), strict=strict) def _set_credentials(self, loader): ''' 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 new file mode 100644 index 00000000000..58dff9b8fb6 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags.yml @@ -0,0 +1,62 @@ +--- +- hosts: 127.0.0.1 + connection: local + gather_facts: false + environment: "{{ ansible_test.environment }}" + tasks: + + - module_defaults: + group/aws: + aws_access_key: '{{ aws_access_key }}' + aws_secret_key: '{{ aws_secret_key }}' + security_token: '{{ security_token | default(omit) }}' + region: '{{ aws_region }}' + block: + + # Create VPC, subnet, security group, and find image_id to create instance + - include_tasks: setup.yml + + # Create new host + - name: create a new host + ec2_instance: + image_id: '{{ image_id }}' + name: '{{ resource_prefix }}' + tags: + Tag1: Test1 + Tag2: Test2 + instance_type: t2.micro + security_groups: '{{ sg_id }}' + vpc_subnet_id: '{{ subnet_id }}' + wait: false + register: setup_instance + + # refresh inventory + - meta: refresh_inventory + + - debug: + var: groups + + - name: assert groups and hostvars were populated with inventory + assert: + that: + - "'aws_ec2' in groups" + - groups['aws_ec2'] | length == 2 + - "'Tag1_Test1' in groups['aws_ec2']" + - "'Tag2_Test2' in groups['aws_ec2']" + - "'Tag1_Test1' in hostvars" + - "'Tag2_Test2' in hostvars" + + always: + + - name: remove ec2 instance + ec2_instance: + instance_type: t2.micro + instance_ids: '{{ setup_instance.instance_ids }}' + state: absent + name: '{{ resource_prefix }}' + security_groups: "{{ sg_id }}" + vpc_subnet_id: "{{ subnet_id }}" + ignore_errors: true + when: setup_instance is defined + + - include_tasks: tear_down.yml 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 new file mode 100644 index 00000000000..639acca007e --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml @@ -0,0 +1,62 @@ +--- +- hosts: 127.0.0.1 + connection: local + gather_facts: false + environment: "{{ ansible_test.environment }}" + tasks: + + - module_defaults: + group/aws: + aws_access_key: '{{ aws_access_key }}' + aws_secret_key: '{{ aws_secret_key }}' + security_token: '{{ security_token | default(omit) }}' + region: '{{ aws_region }}' + block: + + # Create VPC, subnet, security group, and find image_id to create instance + - include_tasks: setup.yml + + # Create new host + - name: create a new host + ec2_instance: + image_id: '{{ image_id }}' + name: '{{ resource_prefix }}' + tags: + Tag1: Test1 + Tag2: Test2 + instance_type: t2.micro + security_groups: '{{ sg_id }}' + vpc_subnet_id: '{{ subnet_id }}' + wait: false + register: setup_instance + + # refresh inventory + - meta: refresh_inventory + + - debug: + var: groups + + - name: assert groups and hostvars were populated with inventory + assert: + that: + - "'aws_ec2' in groups" + - groups['aws_ec2'] | length == 2 + - "'Test1' in groups['aws_ec2']" + - "'Test2' in groups['aws_ec2']" + - "'Test1' in hostvars" + - "'Test2' in hostvars" + + always: + + - name: remove ec2 instance + ec2_instance: + instance_type: t2.micro + instance_ids: '{{ setup_instance.instance_ids }}' + state: absent + name: '{{ resource_prefix }}' + security_groups: "{{ sg_id }}" + vpc_subnet_id: "{{ subnet_id }}" + ignore_errors: true + when: setup_instance is defined + + - include_tasks: tear_down.yml diff --git a/tests/integration/targets/inventory_aws_ec2/runme.sh b/tests/integration/targets/inventory_aws_ec2/runme.sh index 216994672f3..d2940cd2a37 100755 --- a/tests/integration/targets/inventory_aws_ec2/runme.sh +++ b/tests/integration/targets/inventory_aws_ec2/runme.sh @@ -40,6 +40,10 @@ ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_w ansible-playbook playbooks/test_populating_inventory_with_concatenation.yml "$@" ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_literal_string.yml.j2'" "$@" ansible-playbook playbooks/test_populating_inventory_with_literal_string.yml "$@" +ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_hostnames_using_tags_classic.yml.j2'" "$@" +ansible-playbook playbooks/test_populating_inventory_with_hostnames_using_tags_classic.yml "$@" +ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_hostnames_using_tags.yml.j2'" "$@" +ansible-playbook playbooks/test_populating_inventory_with_hostnames_using_tags.yml "$@" # generate inventory config with includes_entries_matching and prepare the tests ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_include_or_exclude_filters.yml.j2'" "$@" diff --git a/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags.yml.j2 b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags.yml.j2 new file mode 100644 index 00000000000..2f7882a2289 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags.yml.j2 @@ -0,0 +1,21 @@ +plugin: amazon.aws.aws_ec2 +aws_access_key_id: '{{ aws_access_key }}' +aws_secret_access_key: '{{ aws_secret_key }}' +{% if security_token | default(false) %} +aws_security_token: '{{ security_token }}' +{% endif %} +regions: +- '{{ aws_region }}' +keyed_groups: +- prefix: tag + key: tags +hostnames: +# can also be specified using +# - tag:Tag1,Tag2 +# or +# - tag:Tag1 +# - tag:Tag2 +# or +- tag:Tag1=Test1,Tag2=Test2 +compose: + ansible_host: private_ip_address diff --git a/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags_classic.yml.j2 b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags_classic.yml.j2 new file mode 100644 index 00000000000..3138a4a2af1 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_hostnames_using_tags_classic.yml.j2 @@ -0,0 +1,21 @@ +plugin: amazon.aws.aws_ec2 +aws_access_key_id: '{{ aws_access_key }}' +aws_secret_access_key: '{{ aws_secret_key }}' +{% if security_token | default(false) %} +aws_security_token: '{{ security_token }}' +{% endif %} +regions: +- '{{ aws_region }}' +keyed_groups: +- prefix: tag + key: tags +hostnames: +# can also be specified using +# - tag:Tag1,Tag2 +# or +# - tag:Tag1=Test1,Tag2=Test2 +# or +- tag:Tag1 +- tag:Tag2 +compose: + ansible_host: private_ip_address diff --git a/tests/unit/plugins/inventory/test_aws_ec2.py b/tests/unit/plugins/inventory/test_aws_ec2.py index 3cdf228c902..c27e84ccdcf 100644 --- a/tests/unit/plugins/inventory/test_aws_ec2.py +++ b/tests/unit/plugins/inventory/test_aws_ec2.py @@ -145,19 +145,19 @@ def test_boto3_conn(inventory): def test_get_hostname_default(inventory): instance = instances['Instances'][0] - assert inventory._get_hostname(instance, hostnames=None) == "ec2-12-345-67-890.compute-1.amazonaws.com" + assert inventory._get_hostname(instance, hostnames=None)[0] == "ec2-12-345-67-890.compute-1.amazonaws.com" def test_get_hostname(inventory): hostnames = ['ip-address', 'dns-name'] instance = instances['Instances'][0] - assert inventory._get_hostname(instance, hostnames) == "12.345.67.890" + assert inventory._get_hostname(instance, hostnames)[0] == "12.345.67.890" def test_get_hostname_dict(inventory): hostnames = [{'name': 'private-ip-address', 'separator': '_', 'prefix': 'tag:Name'}] instance = instances['Instances'][0] - assert inventory._get_hostname(instance, hostnames) == "aws_ec2_098.76.54.321" + assert inventory._get_hostname(instance, hostnames)[0] == "aws_ec2_098.76.54.321" def test_set_credentials(inventory):