From f5727fb8d2be8eb3355f49cc6c851bc94d56da35 Mon Sep 17 00:00:00 2001 From: abikouo Date: Mon, 13 Feb 2023 11:11:14 +0100 Subject: [PATCH 1/2] inventory aws_ec2 - add support for AND logic on exclude and include filters --- ..._aws_ec2-support-and-logic-for-filters.yml | 3 + plugins/inventory/aws_ec2.py | 102 +++++++++++++----- .../playbooks/manage_ec2_instances.yml | 2 +- ...ng_inventory_with_AND_logic_on_filters.yml | 21 ++++ .../playbooks/vars/filters_case1.yml | 8 ++ .../playbooks/vars/filters_case2.yml | 5 + .../playbooks/vars/filters_case3.yml | 6 ++ .../targets/inventory_aws_ec2/runme.sh | 7 ++ .../targets/inventory_aws_ec2/tasks/setup.yml | 4 +- ...ude_exclude_filters_and_apply_logic.yml.j2 | 17 +++ tests/unit/plugins/inventory/test_aws_ec2.py | 60 +++++++++++ 11 files changed, 208 insertions(+), 27 deletions(-) create mode 100644 changelogs/fragments/1361-inventory_aws_ec2-support-and-logic-for-filters.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_AND_logic_on_filters.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case1.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case2.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case3.yml create mode 100644 tests/integration/targets/inventory_aws_ec2/templates/inventory_with_include_exclude_filters_and_apply_logic.yml.j2 diff --git a/changelogs/fragments/1361-inventory_aws_ec2-support-and-logic-for-filters.yml b/changelogs/fragments/1361-inventory_aws_ec2-support-and-logic-for-filters.yml new file mode 100644 index 00000000000..0c999fd959c --- /dev/null +++ b/changelogs/fragments/1361-inventory_aws_ec2-support-and-logic-for-filters.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - inventory aws ec2 - support AND logic when using exclude and/or include filters (https://github.com/ansible-collections/amazon.aws/issues/1354). diff --git a/plugins/inventory/aws_ec2.py b/plugins/inventory/aws_ec2.py index eb0271a901f..9547512d0ee 100644 --- a/plugins/inventory/aws_ec2.py +++ b/plugins/inventory/aws_ec2.py @@ -75,8 +75,8 @@ description: - A list of filters. Any instances matching at least one of the filters are included in the result. - Available filters are listed here U(http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html#options). - - Every entry in this list triggers a search query. As such, from a performance point of view, it's better to - keep the list as short as possible. + - Every entry in this list triggers a search query if C(apply_and_logic_on_include_filters) is not set to I(true). + As such, from a performance point of view, it's better to keep the list as short as possible. type: list elements: dict default: [] @@ -86,8 +86,8 @@ - A list of filters. Any instances matching one of the filters are excluded from the result. - The filters from C(exclude_filters) take priority over the C(include_filters) and C(filters) keys - Available filters are listed here U(http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html#options). - - Every entry in this list triggers a search query. As such, from a performance point of view, it's better to - keep the list as short as possible. + - Every entry in this list triggers a search query if C(apply_and_logic_on_exclude_filters) is not set to I(true). + As such, from a performance point of view, it's better to keep the list as short as possible. type: list elements: dict default: [] @@ -141,6 +141,22 @@ type: bool default: False version_added: 6.0.0 + apply_and_logic_on_include_filters: + description: + - When multiple filters are specified on the C(include_filters) and C(filters) keys, + the filters are joined with an AND, if this parameter is set to I(true) + therefore only instances matching all of the specified filters are returned in the result. + type: bool + version_added: 6.0.0 + default: False + apply_and_logic_on_exclude_filters: + description: + - When multiple filters are specified on the C(exclude_filters), + the filters are joined with an AND, if this parameter is set to I(true) + therefore any instances matching all of the filters are excluded from the result. + type: bool + version_added: 6.0.0 + default: False """ EXAMPLES = r""" @@ -259,6 +275,16 @@ - us-east-1 hostvars_prefix: 'aws_' hostvars_suffix: '_ec2' + +# Example using include_filters and apply_and_logic_on_include_filters to compose the inventory. +plugin: aws_ec2 +regions: + - us-east-1 + - us-west-1 +include_filters: +- tag-key: Role +- tag-key: Team +apply_and_logic_on_include_filters: true """ import re @@ -595,28 +621,43 @@ def _get_all_hostnames(self, instance, hostnames): return hostname_list - def _query(self, regions, include_filters, exclude_filters, strict_permissions, use_ssm_inventory): - ''' - :param regions: a list of regions to query - :param include_filters: a list of boto3 filter dictionaries - :param exclude_filters: a list of boto3 filter dictionaries - :param strict_permissions: a boolean determining whether to fail or ignore 403 error codes - - ''' + def _query( + self, + regions, + include_filters, + exclude_filters, + strict_permissions, + use_ssm_inventory, + include_filter_and_logic, + exclude_filter_and_logic, + ): + """ + :param regions: a list of regions to query + :param include_filters: a list of boto3 filter dictionaries + :param exclude_filters: a list of boto3 filter dictionaries + :param strict_permissions: a boolean determining whether to fail or ignore 403 error codes + + """ instances = [] ids_to_ignore = [] - for filter in exclude_filters: - for i in self._get_instances_by_region( - regions, - ansible_dict_to_boto3_filter_list(filter), - strict_permissions): - ids_to_ignore.append(i['InstanceId']) - for filter in include_filters: - for i in self._get_instances_by_region( - regions, - ansible_dict_to_boto3_filter_list(filter), - strict_permissions): - if i['InstanceId'] not in ids_to_ignore: + + def _build_aws_filters(filters_opts, apply_and_logic): + aws_filters = [] + for filter in filters_opts: + aws_boto3_filter = ansible_dict_to_boto3_filter_list(filter) + if apply_and_logic: + aws_filters += aws_boto3_filter + else: + aws_filters.append(aws_boto3_filter) + return [aws_filters] if apply_and_logic else aws_filters + + for filter in _build_aws_filters(exclude_filters, exclude_filter_and_logic): + for i in self._get_instances_by_region(regions, filter, strict_permissions): + ids_to_ignore.append(i["InstanceId"]) + + for filter in _build_aws_filters(include_filters, include_filter_and_logic): + for i in self._get_instances_by_region(regions, filter, strict_permissions): + if i["InstanceId"] not in ids_to_ignore: instances.append(i) ids_to_ignore.append(i['InstanceId']) @@ -727,6 +768,9 @@ def parse(self, inventory, loader, path, cache=True): strict_permissions = self.get_option('strict_permissions') allow_duplicated_hosts = self.get_option('allow_duplicated_hosts') + include_filter_and_logic = self.get_option("apply_and_logic_on_include_filters") + exclude_filter_and_logic = self.get_option("apply_and_logic_on_exclude_filters") + hostvars_prefix = self.get_option("hostvars_prefix") hostvars_suffix = self.get_option("hostvars_suffix") use_contrib_script_compatible_ec2_tag_keys = self.get_option("use_contrib_script_compatible_ec2_tag_keys") @@ -741,7 +785,15 @@ def parse(self, inventory, loader, path, cache=True): result_was_cached, results = self.get_cached_result(path, cache) if not result_was_cached: - results = self._query(regions, include_filters, exclude_filters, strict_permissions, use_ssm_inventory) + results = self._query( + regions, + include_filters, + exclude_filters, + strict_permissions, + use_ssm_inventory, + include_filter_and_logic, + exclude_filter_and_logic, + ) self._populate( results, diff --git a/tests/integration/targets/inventory_aws_ec2/playbooks/manage_ec2_instances.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/manage_ec2_instances.yml index acef915cbb0..4ac80ebb622 100644 --- a/tests/integration/targets/inventory_aws_ec2/playbooks/manage_ec2_instances.yml +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/manage_ec2_instances.yml @@ -1,7 +1,7 @@ --- - hosts: 127.0.0.1 connection: local - gather_facts: no + gather_facts: false collections: - amazon.aws diff --git a/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_AND_logic_on_filters.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_AND_logic_on_filters.yml new file mode 100644 index 00000000000..f7810c003ee --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/test_populating_inventory_with_AND_logic_on_filters.yml @@ -0,0 +1,21 @@ +--- +- hosts: 127.0.0.1 + connection: local + gather_facts: false + environment: "{{ ansible_test.environment }}" + tasks: + + - meta: refresh_inventory + + - name: "Ensure we've got a hostvars entry for the new host {{ item }}" + assert: + that: + - item in hostvars + with_items: "{{ test_inventory_hosts }}" + when: test_inventory_hosts | default([]) | length > 0 + + - name: "Ensure we've got no hosts from inventory" + assert: + that: + - hostvars.keys() | list | length == 0 + when: test_inventory_hosts | default([]) | length == 0 diff --git a/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case1.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case1.yml new file mode 100644 index 00000000000..8dd3800ec13 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case1.yml @@ -0,0 +1,8 @@ +--- +test_inventory_hosts: + - "{{ resource_prefix }}" +test_include_filters: + - tag-key: resource_prefix +test_exclude_filters: + - tag-key: resource_prefix + - tag-key: this_tag_does_not_exist \ No newline at end of file diff --git a/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case2.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case2.yml new file mode 100644 index 00000000000..94e0d635651 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case2.yml @@ -0,0 +1,5 @@ +--- +test_inventory_hosts: [] +test_exclude_filters: + - tag-key: resource_prefix + - "tag:Name": "{{ resource_prefix }}" \ No newline at end of file diff --git a/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case3.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case3.yml new file mode 100644 index 00000000000..884b8f7ce16 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/vars/filters_case3.yml @@ -0,0 +1,6 @@ +--- +test_inventory_hosts: + - '{{ resource_prefix }}' +test_exclude_filters: + - tag-key: resource_prefix + - tag-key: this_tag_does_not_exist \ No newline at end of file diff --git a/tests/integration/targets/inventory_aws_ec2/runme.sh b/tests/integration/targets/inventory_aws_ec2/runme.sh index 4423e21f422..0445f9164b1 100755 --- a/tests/integration/targets/inventory_aws_ec2/runme.sh +++ b/tests/integration/targets/inventory_aws_ec2/runme.sh @@ -65,6 +65,13 @@ ansible-playbook playbooks/test_populating_inventory_with_hostvars_prefix_suffix ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_include_or_exclude_filters.yml.j2'" "$@" ansible-playbook playbooks/test_populating_inventory_with_include_or_exclude_filters.yml "$@" +# generate inventory config with and logic for include/exclude filters +for var_file in playbooks/vars/filters_*.yml +do + ansible-playbook playbooks/create_inventory_config.yml -e "@${var_file}" -e "template='inventory_with_include_exclude_filters_and_apply_logic.yml.j2'" "$@" + ansible-playbook playbooks/test_populating_inventory_with_AND_logic_on_filters.yml -e "@${var_file}" "$@" +done + # generate inventory config with caching and test using it ansible-playbook playbooks/create_inventory_config.yml -e "template='inventory_with_use_contrib_script_keys.yml.j2'" "$@" ANSIBLE_TRANSFORM_INVALID_GROUP_CHARS=never ansible-playbook playbooks/test_populating_inventory_with_use_contrib_script_keys.yml "$@" diff --git a/tests/integration/targets/inventory_aws_ec2/tasks/setup.yml b/tests/integration/targets/inventory_aws_ec2/tasks/setup.yml index 0a556b18a9f..8adc64c817e 100644 --- a/tests/integration/targets/inventory_aws_ec2/tasks/setup.yml +++ b/tests/integration/targets/inventory_aws_ec2/tasks/setup.yml @@ -57,5 +57,7 @@ instance_type: t2.micro security_groups: '{{ sg_id }}' vpc_subnet_id: '{{ subnet_id }}' - wait: no + wait: false + resource_tags: + resource_prefix: '{{ resource_prefix }}' register: setup_instance diff --git a/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_include_exclude_filters_and_apply_logic.yml.j2 b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_include_exclude_filters_and_apply_logic.yml.j2 new file mode 100644 index 00000000000..e8a50a18b9b --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_include_exclude_filters_and_apply_logic.yml.j2 @@ -0,0 +1,17 @@ +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 }}' +filters: + tag:Name: + - {{ resource_prefix }} +include_filters: {{ test_include_filters | default([]) }} +exclude_filters: {{ test_exclude_filters | default([]) }} +hostnames: +- tag:Name +apply_and_logic_on_include_filters: true +apply_and_logic_on_exclude_filters: true diff --git a/tests/unit/plugins/inventory/test_aws_ec2.py b/tests/unit/plugins/inventory/test_aws_ec2.py index 2bd526eedeb..bad40cd247e 100644 --- a/tests/unit/plugins/inventory/test_aws_ec2.py +++ b/tests/unit/plugins/inventory/test_aws_ec2.py @@ -441,6 +441,8 @@ def test_inventory_query(inventory, include_filters, exclude_filters, instances_ "include_filters": [], "exclude_filters": [], "use_ssm_inventory": False, + "include_filter_and_logic": False, + "exclude_filter_and_logic": False, } for u in include_filters: @@ -454,6 +456,64 @@ def test_inventory_query(inventory, include_filters, exclude_filters, instances_ inventory._get_instances_by_region.assert_not_called() +def test_inventory_query_include_filters_with_and_logic(inventory): + instances = [{"InstanceId": 2, "name": "instance-2"}] + + inventory._get_instances_by_region = MagicMock() + inventory._get_instances_by_region.side_effect = [instances] + + regions = ["some-region", "another-region"] + strict = True + + include_filters = [ + {"tag-key": "environment"}, + {"tag:Project": "Ansible"}, + ] + + params = { + "regions": regions, + "strict_permissions": strict, + "include_filters": include_filters, + "exclude_filters": [], + "include_filter_and_logic": True, + "exclude_filter_and_logic": False, + } + + assert inventory._query(**params) == {"aws_ec2": instances} + + aws_filters = [{"Name": "tag-key", "Values": ["environment"]}, {"Name": "tag:Project", "Values": ["Ansible"]}] + inventory._get_instances_by_region.assert_called_once_with(regions, aws_filters, strict) + + +def test_inventory_query_exclude_filters_with_and_logic(inventory): + instances = [{"InstanceId": 2, "name": "instance-2"}] + + inventory._get_instances_by_region = MagicMock() + inventory._get_instances_by_region.side_effect = [instances] + + regions = ["some-region", "another-region"] + strict = True + + exclude_filters = [ + {"tag-key": "environment"}, + {"tag:Project": "Ansible"}, + ] + + params = { + "regions": regions, + "strict_permissions": strict, + "include_filters": [], + "exclude_filters": exclude_filters, + "include_filter_and_logic": False, + "exclude_filter_and_logic": True, + } + + assert inventory._query(**params) == {"aws_ec2": []} + + aws_filters = [{"Name": "tag-key", "Values": ["environment"]}, {"Name": "tag:Project", "Values": ["Ansible"]}] + inventory._get_instances_by_region.assert_called_once_with(regions, aws_filters, strict) + + @pytest.mark.parametrize( "filters", [ From 81a8707fc2cf8630685b25dab96084b101d40c0d Mon Sep 17 00:00:00 2001 From: abikouo Date: Wed, 8 Mar 2023 15:05:18 +0100 Subject: [PATCH 2/2] fix unit tests issue --- tests/unit/plugins/inventory/test_aws_ec2.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/plugins/inventory/test_aws_ec2.py b/tests/unit/plugins/inventory/test_aws_ec2.py index bad40cd247e..2c37a7ed56a 100644 --- a/tests/unit/plugins/inventory/test_aws_ec2.py +++ b/tests/unit/plugins/inventory/test_aws_ec2.py @@ -475,6 +475,7 @@ def test_inventory_query_include_filters_with_and_logic(inventory): "strict_permissions": strict, "include_filters": include_filters, "exclude_filters": [], + "use_ssm_inventory": False, "include_filter_and_logic": True, "exclude_filter_and_logic": False, } @@ -502,6 +503,7 @@ def test_inventory_query_exclude_filters_with_and_logic(inventory): params = { "regions": regions, "strict_permissions": strict, + "use_ssm_inventory": False, "include_filters": [], "exclude_filters": exclude_filters, "include_filter_and_logic": False,