Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inventory aws_ec2 - add support for AND logic #1365

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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).
102 changes: 77 additions & 25 deletions plugins/inventory/aws_ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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).
abikouo marked this conversation as resolved.
Show resolved Hide resolved
As such, from a performance point of view, it's better to keep the list as short as possible.
type: list
elements: dict
default: []
Expand All @@ -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: []
Expand Down Expand Up @@ -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"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Comment on lines +644 to +652
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like something which should be pulled up a level with unit tests added.


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'])

Expand Down Expand Up @@ -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")
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
- hosts: 127.0.0.1
connection: local
gather_facts: no
gather_facts: false

collections:
- amazon.aws
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
test_inventory_hosts: []
test_exclude_filters:
- tag-key: resource_prefix
- "tag:Name": "{{ resource_prefix }}"
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
test_inventory_hosts:
- '{{ resource_prefix }}'
test_exclude_filters:
- tag-key: resource_prefix
- tag-key: this_tag_does_not_exist
7 changes: 7 additions & 0 deletions tests/integration/targets/inventory_aws_ec2/runme.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 "$@"
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/targets/inventory_aws_ec2/tasks/setup.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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
62 changes: 62 additions & 0 deletions tests/unit/plugins/inventory/test_aws_ec2.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -454,6 +456,66 @@ 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": [],
"use_ssm_inventory": False,
"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,
"use_ssm_inventory": False,
"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",
[
Expand Down