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 <hebailey@redhat.com>
Reviewed-by: Alina Buzachis

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:

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') }}"

tests/integration/targets/inventory_aws_ec2/templates/config.ini.j2

@@ -0,0 +1,3 @@
+[ansible-test]
+
+region = {{ aws_region }}

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 }}'

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)

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