From 89ec6ba2ee7fae84eb1aae098da040eba4974c7d Mon Sep 17 00:00:00 2001 From: Bikouo Aubin <79859644+abikouo@users.noreply.github.com> Date: Mon, 4 Mar 2024 18:27:17 +0100 Subject: [PATCH] Inventory plugins do not convert template parameters (#1980) Inventory plugins do not convert template parameters SUMMARY Fixes #1955 ISSUE TYPE Bugfix Pull Request COMPONENT NAME Inventory plugin Reviewed-by: Brian A. Teller Reviewed-by: Helen Bailey Reviewed-by: Alina Buzachis --- plugins/plugin_utils/inventory.py | 36 ++--- .../playbooks/create_inventory_config.yml | 5 + .../inventory_aws_ec2/templates/config.ini.j2 | 3 + .../templates/inventory_with_template.yml.j2 | 2 +- .../inventory/test_inventory_base.py | 131 ++++++++++++++++++ tests/unit/plugins/inventory/test_aws_ec2.py | 1 + 6 files changed, 160 insertions(+), 18 deletions(-) create mode 100644 tests/integration/targets/inventory_aws_ec2/templates/config.ini.j2 diff --git a/plugins/plugin_utils/inventory.py b/plugins/plugin_utils/inventory.py index 144f77a7a50..b0e47f7ef55 100644 --- a/plugins/plugin_utils/inventory.py +++ b/plugins/plugin_utils/inventory.py @@ -33,7 +33,10 @@ class TemplatedOptions: "secret_key", "session_token", "profile", - "iam_role_name", + "endpoint_url", + "assume_role_arn", + "region", + "regions", ) def __init__(self, templar, options): @@ -48,20 +51,21 @@ def __setitem__(self, *args): def get(self, *args): value = self.original_options.get(*args) - if not value: - return value - if args[0] not in self.TEMPLATABLE_OPTIONS: - return value - if not self.templar.is_template(value): + if ( + not value + or not self.templar + or args[0] not in self.TEMPLATABLE_OPTIONS + or not self.templar.is_template(value) + ): return value return self.templar.template(variable=value, disable_lookups=False) def get_options(self, *args): - original_options = super().get_options(*args) - if not self.templar: - return original_options - return self.TemplatedOptions(self.templar, original_options) + return self.TemplatedOptions(self.templar, super().get_options(*args)) + + def get_option(self, option, hostvars=None): + return self.TemplatedOptions(self.templar, {option: super().get_option(option, hostvars)}).get(option) def __init__(self): super().__init__() @@ -109,8 +113,7 @@ def _freeze_iam_role(self, iam_role_arn): } def _set_frozen_credentials(self): - options = self.get_options() - iam_role_arn = options.get("assume_role_arn") + iam_role_arn = self.get_option("assume_role_arn") if iam_role_arn: self._freeze_iam_role(iam_role_arn) @@ -136,10 +139,9 @@ def _describe_regions(self, service): return None def _boto3_regions(self, service): - options = self.get_options() - - if options.get("regions"): - return options.get("regions") + regions = self.get_option("regions") + if regions: + return regions # boto3 has hard coded lists of available regions for resources, however this does bit-rot # As such we try to query the service, and fall back to ec2 for a list of regions @@ -149,7 +151,7 @@ def _boto3_regions(self, service): return regions # fallback to local list hardcoded in boto3 if still no regions - session = _boto3_session(options.get("profile")) + session = _boto3_session(self.get_option("profile")) regions = session.get_available_regions(service) if not regions: diff --git a/tests/integration/targets/inventory_aws_ec2/playbooks/create_inventory_config.yml b/tests/integration/targets/inventory_aws_ec2/playbooks/create_inventory_config.yml index 232911d2453..282ca43ee86 100644 --- a/tests/integration/targets/inventory_aws_ec2/playbooks/create_inventory_config.yml +++ b/tests/integration/targets/inventory_aws_ec2/playbooks/create_inventory_config.yml @@ -9,3 +9,8 @@ ansible.builtin.copy: dest: ../test.aws_ec2.yml content: "{{ lookup('template', template_name) }}" + + - name: write ini configuration + ansible.builtin.copy: + dest: ../config.ini + content: "{{ lookup('template', '../templates/config.ini.j2') }}" diff --git a/tests/integration/targets/inventory_aws_ec2/templates/config.ini.j2 b/tests/integration/targets/inventory_aws_ec2/templates/config.ini.j2 new file mode 100644 index 00000000000..f7320a7fbf4 --- /dev/null +++ b/tests/integration/targets/inventory_aws_ec2/templates/config.ini.j2 @@ -0,0 +1,3 @@ +[ansible-test] + +region = {{ aws_region }} \ No newline at end of file diff --git a/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_template.yml.j2 b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_template.yml.j2 index 44a132c1c53..dee7422a9c9 100644 --- a/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_template.yml.j2 +++ b/tests/integration/targets/inventory_aws_ec2/templates/inventory_with_template.yml.j2 @@ -5,7 +5,7 @@ secret_key: '{{ aws_secret_key }}' session_token: '{{ security_token }}' {% endif %} regions: -- '{{ aws_region }}' +- '{{ '{{ lookup("ansible.builtin.ini", "region", section="ansible-test", file="config.ini") }}' }}' filters: tag:Name: - '{{ resource_prefix }}' diff --git a/tests/unit/plugin_utils/inventory/test_inventory_base.py b/tests/unit/plugin_utils/inventory/test_inventory_base.py index 32eb3f7abf4..4da5792a8d4 100644 --- a/tests/unit/plugin_utils/inventory/test_inventory_base.py +++ b/tests/unit/plugin_utils/inventory/test_inventory_base.py @@ -3,6 +3,7 @@ # This file is part of Ansible # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +import re from unittest.mock import MagicMock from unittest.mock import call from unittest.mock import patch @@ -11,6 +12,8 @@ import pytest import ansible.plugins.inventory as base_inventory +from ansible.errors import AnsibleError +from ansible.module_utils.six import string_types import ansible_collections.amazon.aws.plugins.plugin_utils.inventory as utils_inventory @@ -65,3 +68,131 @@ def test_inventory_verify_file(monkeypatch, filename, result): assert inventory_plugin.verify_file(filename) is result base_verify.return_value = False assert inventory_plugin.verify_file(filename) is False + + +class AwsUnitTestTemplar: + def __init__(self, config): + self.config = config + + def is_template_string(self, key): + m = re.findall("{{([ ]*[a-zA-Z0-9_]*[ ]*)}}", key) + return bool(m) + + def is_template(self, data): + if isinstance(data, string_types): + return self.is_template_string(data) + elif isinstance(data, (list, tuple)): + for v in data: + if self.is_template(v): + return True + elif isinstance(data, dict): + for k in data: + if self.is_template(k) or self.is_template(data[k]): + return True + return False + + def template(self, variable, disable_lookups): + for k, v in self.config.items(): + variable = re.sub("{{([ ]*%s[ ]*)}}" % k, v, variable) + if self.is_template_string(variable): + m = re.findall("{{([ ]*[a-zA-Z0-9_]*[ ]*)}}", variable) + raise AnsibleError(f"Missing variables: {','.join([k.replace(' ', '') for k in m])}") + return variable + + +@pytest.fixture +def aws_inventory_base(): + inventory = utils_inventory.AWSInventoryBase() + inventory._options = {} + inventory.templar = None + return inventory + + +@pytest.mark.parametrize( + "option,value", + [ + ("access_key", "amazon_ansible_access_key_001"), + ("secret_key", "amazon_ansible_secret_key_890"), + ("session_token", None), + ("use_ssm_inventory", False), + ("This_field_is_undefined", None), + ("assume_role_arn", "arn:aws:iam::123456789012:role/ansible-test-inventory"), + ("region", "us-east-2"), + ], +) +def test_inventory_get_options_without_templar(aws_inventory_base, mocker, option, value): + inventory_options = { + "access_key": "amazon_ansible_access_key_001", + "secret_key": "amazon_ansible_secret_key_890", + "endpoint": "http//ansible.amazon.com", + "assume_role_arn": "arn:aws:iam::123456789012:role/ansible-test-inventory", + "region": "us-east-2", + "use_ssm_inventory": False, + } + aws_inventory_base._options = inventory_options + + super_get_options_patch = mocker.patch( + "ansible_collections.amazon.aws.plugins.plugin_utils.inventory.BaseInventoryPlugin.get_options" + ) + super_get_options_patch.return_value = aws_inventory_base._options + + options = aws_inventory_base.get_options() + assert value == options.get(option) + + +@pytest.mark.parametrize( + "option,value,error", + [ + ("access_key", "amazon_ansible_access_key_001", None), + ("session_token", None, None), + ("use_ssm_inventory", "{{ aws_inventory_use_ssm }}", None), + ("This_field_is_undefined", None, None), + ("region", "us-east-1", None), + ("profile", None, "Missing variables: ansible_version"), + ], +) +def test_inventory_get_options_with_templar(aws_inventory_base, mocker, option, value, error): + inventory_options = { + "access_key": "amazon_ansible_access_key_001", + "profile": "ansbile_{{ ansible_os }}_{{ ansible_version }}", + "endpoint": "{{ aws_endpoint }}", + "region": "{{ aws_region_country }}-east-{{ aws_region_id }}", + "use_ssm_inventory": "{{ aws_inventory_use_ssm }}", + } + aws_inventory_base._options = inventory_options + templar_config = { + "ansible_os": "RedHat", + "aws_region_country": "us", + "aws_region_id": "1", + "aws_endpoint": "http//ansible.amazon.com", + } + aws_inventory_base.templar = AwsUnitTestTemplar(templar_config) + + super_get_options_patch = mocker.patch( + "ansible_collections.amazon.aws.plugins.plugin_utils.inventory.BaseInventoryPlugin.get_options" + ) + super_get_options_patch.return_value = aws_inventory_base._options + + super_get_option_patch = mocker.patch( + "ansible_collections.amazon.aws.plugins.plugin_utils.inventory.BaseInventoryPlugin.get_option" + ) + super_get_option_patch.side_effect = lambda x, hostvars=None: aws_inventory_base._options.get(x) + + if error: + # test using get_options() + with pytest.raises(AnsibleError) as exc: + options = aws_inventory_base.get_options() + options.get(option) + assert error == str(exc.value) + + # test using get_option() + with pytest.raises(AnsibleError) as exc: + aws_inventory_base.get_option(option) + assert error == str(exc.value) + else: + # test using get_options() + options = aws_inventory_base.get_options() + assert value == options.get(option) + + # test using get_option() + assert value == aws_inventory_base.get_option(option) diff --git a/tests/unit/plugins/inventory/test_aws_ec2.py b/tests/unit/plugins/inventory/test_aws_ec2.py index 8cced16621e..e33b78c5102 100644 --- a/tests/unit/plugins/inventory/test_aws_ec2.py +++ b/tests/unit/plugins/inventory/test_aws_ec2.py @@ -240,6 +240,7 @@ def test_get_tag_hostname(preference, instance, expected): ) def test_inventory_build_include_filters(inventory, _options, expected): inventory._options = _options + inventory.templar = None assert inventory.build_include_filters() == expected