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

ipaddr is too eager to evaluate variables types #299

Open
cosandr opened this issue Aug 4, 2023 · 0 comments
Open

ipaddr is too eager to evaluate variables types #299

cosandr opened this issue Aug 4, 2023 · 0 comments

Comments

@cosandr
Copy link

cosandr commented Aug 4, 2023

SUMMARY

Please excuse bad title, I really don't know what to call this. The issue is fairly straight forward to test for. This could affect other modules, but I've only experienced it with the ipaddr filter. The problem is with the input validation being too eager, most clearly visible when trying to use hostvars (see replication steps below).

This patch fixes the issue but it changes the behavior at runtime as well, although I'd argue it's more consistent with other modules.

diff --git a/plugins/filter/ipaddr.py b/plugins/filter/ipaddr.py
index 0ed9865..3e87309 100644
--- a/plugins/filter/ipaddr.py
+++ b/plugins/filter/ipaddr.py
@@ -11,6 +11,7 @@ from __future__ import absolute_import, division, print_function
 from functools import partial

 from ansible.errors import AnsibleFilterError
+from ansible.template import AnsibleUndefined

 from ansible_collections.ansible.utils.plugins.module_utils.common.argspec_validate import (
     AnsibleArgSpecValidator,
@@ -263,6 +264,8 @@ def _ipaddr(*args, **kwargs):
             pass
         elif isinstance(data["value"], int):
             pass
+        elif isinstance(data["value"], AnsibleUndefined):
+            pass
         else:
             raise AnsibleFilterError(
                 "Unrecognized type <{0}> for ipaddr filter <{1}>".format(

With this patch you will get "test": "VARIABLE IS NOT DEFINED!" at runtime instead of an exception.

The output of hostvars with the patch is consistent with other modules, they're displayed as is (in raw form) until actually used at runtime where they're evaluated. Someone more familiar with the code base may decide if this is the right way to fix it, if it is I'd gladly open a PR if desired.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

ansible.utils.ipaddr

ANSIBLE VERSION
ansible [core 2.15.2]
  config file = /home/andrei/src/ansible-test/ansible.cfg
  configured module search path = ['/home/andrei/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3.11/site-packages/ansible
  ansible collection location = /home/andrei/src/ansible
  executable location = /usr/bin/ansible
  python version = 3.11.3 (main, Jun  5 2023, 09:32:32) [GCC 13.1.1 20230429] (/usr/bin/python)
  jinja version = 3.1.2
  libyaml = True
COLLECTION VERSION
# /home/andrei/src/ansible/ansible_collections
Collection    Version
------------- -------
ansible.utils 2.10.4

# /usr/lib/python3.11/site-packages/ansible_collections
Collection    Version
------------- -------
ansible.utils 2.10.3
CONFIGURATION
COLLECTIONS_PATHS(/home/andrei/src/ansible-test/ansible.cfg) = ['/home/andrei/src/ansible']
CONFIG_FILE() = /home/andrei/src/ansible-test/ansible.cfg
DEFAULT_HOST_LIST(/home/andrei/src/ansible-test/ansible.cfg) = ['/home/andrei/src/ansible-test/hosts.yml']
EDITOR(env: EDITOR) = vim
PAGER(env: PAGER) = less
OS / ENVIRONMENT

Arch, this issue is OS agnostic.

STEPS TO REPRODUCE

Inventory

all:
  hosts:
    a:
      test:
        - "10.0.0.1"
        - "10.0.0.2"
    b:
      test: "{{ hostvars['a'].test | ansible.utils.ipaddr('host') }}"

Config

[defaults]
inventory = hosts.yml
# This is here so I could test my changes, it only contains a clone of this repo's main branch
collections_path = ~/src/ansible
EXPECTED RESULTS
$ ansible b -m debug -a var=test
b | SUCCESS => {
    "test": [
        "10.0.0.1/32",
        "10.0.0.2/32"
    ]
}

$ ansible localhost -m debug -a msg="{{ hostvars['b'] }}"
localhost | SUCCESS => {
    "msg": {
        "ansible_check_mode": false,
        "ansible_config_file": "/home/andrei/src/ansible-test/ansible.cfg",
        "ansible_diff_mode": false,
        "ansible_facts": {},
        "ansible_forks": 5,
        "ansible_inventory_sources": [
            "/home/andrei/src/ansible-test/hosts.yml"
        ],
        "ansible_playbook_python": "/usr/bin/python",
        "ansible_verbosity": 0,
        "ansible_version": {
            "full": "2.15.2",
            "major": 2,
            "minor": 15,
            "revision": 2,
            "string": "2.15.2"
        },
        "group_names": [
            "ungrouped"
        ],
        "groups": {
            "all": [
                "a",
                "b"
            ],
            "ungrouped": [
                "a",
                "b"
            ]
        },
        "inventory_dir": "/home/andrei/src/ansible-test",
        "inventory_file": "/home/andrei/src/ansible-test/hosts.yml",
        "inventory_hostname": "b",
        "inventory_hostname_short": "b",
        "playbook_dir": "/home/andrei/src/ansible-test",
        "test": "{{ hostvars['a'].test | ansible.utils.ipaddr('host') }}"
    }
}
ACTUAL RESULTS
$ ansible b -m debug -a var=test
b | SUCCESS => {
    "test": [
        "10.0.0.1/32",
        "10.0.0.2/32"
    ]
}

$ ansible localhost -m debug -a msg="{{ hostvars['b'] }}"
localhost | FAILED! => {
    "msg": "Unrecognized type <<class 'ansible.template.AnsibleUndefined'>> for ipaddr filter <value>"
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant