ansible-collections / ansible.utils

A collection of ansible utilities for the content creator.
GNU General Public License v3.0
74 stars 78 forks source link

"Invalid" values for ipaddr filters #240

Open Vladimir-csp opened 1 year ago

Vladimir-csp commented 1 year ago
SUMMARY

Hi. If the point of ipaddr filters is to parse/modify valid values and return False (or discard in lists) for invalid, then why some invalid values produce warnings (and even errors)? The doc contains '' example which is shown to be discarded silently, but in actuality produces a warning.

Chaining multiple ipaddr family and other filters becomes hard as one needs to take care to manually filter out anything that might break it, instead of just relying on ipaddr filter to parse what is parseable. Even chaining two ipaddr filters together will result in warning if the value does not pass through the first because False is now invalid.

This behavior was introduced at some point, but why? and what is the plan for "breaking change in future"?

Just placing this before singular values in ipaddr filters could simplify a lot of stuff:

if isinstance(value, (int, str)) and value:
    # proceed with filtering
    pass
else:
    return False
ISSUE TYPE
COMPONENT NAME

ipaddr

ANSIBLE VERSION
ansible [core 2.14.2]
  config file = None
  configured module search path = ['/home/username/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python3/dist-packages/ansible
  ansible collection location = /home/username/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/bin/ansible
  python version = 3.11.1 (main, Dec 31 2022, 10:23:59) [GCC 12.2.0] (/usr/bin/python3)
  jinja version = 3.0.3
  libyaml = True
COLLECTION VERSION
ansible.utils 2.8.0
CONFIGURATION
CONFIG_FILE() = None
OS / ENVIRONMENT

Debian testing

STEPS TO REPRODUCE
---
- hosts: localhost
  gather_facts: no
  vars:
    potential_ips:
      - 127.0.0.1
      - 192.168.0.0/24
      - 192.168.0.14/24
      - 8.8.8.8
      - 8.8.8.0/27
      - ''
      - some string
      - []
      - yes
      - no
      - 0
      - - 1
        - 2
        - noip
        - ''
        - False
      - 1
      #- 1.12
      #- {key: value}
  tasks:
    - name: ipaddr per item
      debug:
        msg: |-
          {{ item }} => {{ item | ansible.utils.ipaddr }}
      loop: "{{ potential_ips }}"
    - name: ipaddr on list
      debug:
        msg: |-
          {{ potential_ips | ansible.utils.ipaddr }}
EXPECTED RESULTS

Everything invalid is silently discarded, ints and valid stings are parsed

TASK [ipaddr per item] *********************************************************
ok: [localhost] => (item=127.0.0.1) => {
    "msg": "127.0.0.1 => 127.0.0.1"
}
ok: [localhost] => (item=192.168.0.0/24) => {
    "msg": "192.168.0.0/24 => 192.168.0.0/24"
}
ok: [localhost] => (item=192.168.0.14/24) => {
    "msg": "192.168.0.14/24 => 192.168.0.14/24"
}
ok: [localhost] => (item=8.8.8.8) => {
    "msg": "8.8.8.8 => 8.8.8.8"
}
ok: [localhost] => (item=8.8.8.0/27) => {
    "msg": "8.8.8.0/27 => 8.8.8.0/27"
}
ok: [localhost] => (item=) => {
    "msg": " => False"
}
ok: [localhost] => (item=some string) => {
    "msg": "some string => False"
}
ok: [localhost] => (item=[]) => {
    "msg": "[] => []"
}
ok: [localhost] => (item=True) => {
    "msg": "True => False"
}
ok: [localhost] => (item=False) => {
    "msg": "False => False"
}
ok: [localhost] => (item=0) => {
    "msg": "0 => False"
}
ok: [localhost] => (item=[1, 2, 'noip', '', False]) => {
    "msg": "[1, 2, 'noip', '', False] => ['0.0.0.1', '0.0.0.2']"
}
ok: [localhost] => (item=1) => {
    "msg": "1 => 0.0.0.1"
}
ok: [localhost] => (item=1) => {
    "msg": "1.12 => False"
}
ok: [localhost] => (item=1) => {
    "msg": "{key: value} => False"
}
TASK [ipaddr on list] **********************************************************
ok: [localhost] => {
    "msg": [
        "127.0.0.1",
        "192.168.0.0/24",
        "192.168.0.14/24",
        "8.8.8.8",
        "8.8.8.0/27",
        [
            "0.0.0.1",
            "0.0.0.2"
        ],
        "0.0.0.1"
    ]
}
ACTUAL RESULTS
TASK [ipaddr per item] *********************************************************
ok: [localhost] => (item=127.0.0.1) => {
    "msg": "127.0.0.1 => 127.0.0.1"
}
ok: [localhost] => (item=192.168.0.0/24) => {
    "msg": "192.168.0.0/24 => 192.168.0.0/24"
}
ok: [localhost] => (item=192.168.0.14/24) => {
    "msg": "192.168.0.14/24 => 192.168.0.14/24"
}
ok: [localhost] => (item=8.8.8.8) => {
    "msg": "8.8.8.8 => 8.8.8.8"
}
ok: [localhost] => (item=8.8.8.0/27) => {
    "msg": "8.8.8.0/27 => 8.8.8.0/27"
}
[WARNING]: The value '' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
ok: [localhost] => (item=) => {
    "msg": " => False"
}
ok: [localhost] => (item=some string) => {
    "msg": "some string => False"
}
ok: [localhost] => (item=[]) => {
    "msg": "[] => []"
}
[WARNING]: The value 'True' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
ok: [localhost] => (item=True) => {
    "msg": "True => False"
}
[WARNING]: The value 'False' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
ok: [localhost] => (item=False) => {
    "msg": "False => False"
}
[WARNING]: The value '0' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
ok: [localhost] => (item=0) => {
    "msg": "0 => False"
}
ok: [localhost] => (item=[1, 2]) => {
    "msg": "[1, 2] => ['0.0.0.1', '0.0.0.2']"
}
ok: [localhost] => (item=1) => {
    "msg": "1 => 0.0.0.1"
}

TASK [ipaddr on list] **********************************************************
[WARNING]: The value '' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
[WARNING]: The value 'True' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
[WARNING]: The value 'False' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
[WARNING]: The value '0' is not a valid IP address or network, passing this
value to ipaddr filter might result in breaking change in future.
ok: [localhost] => {
    "msg": [
        "127.0.0.1",
        "192.168.0.0/24",
        "192.168.0.14/24",
        "8.8.8.8",
        "8.8.8.0/27",
        [
            "0.0.0.1",
            "0.0.0.2"
        ],
        "0.0.0.1"
    ]
}

If float or dict values are uncommented, it results in error.

fatal: [localhost]: FAILED! => {"msg": "Unrecognized type <<class 'float'>> for ipaddr filter <value>"}
fatal: [localhost]: FAILED! => {"msg": "Unrecognized type <<class 'dict'>> for ipaddr filter <value>"}
Vladimir-csp commented 1 year ago

Another inconsistency: 1 => 0.0.0.1 but 0 => False. Could be 0 => 0.0.0.0

neutralalice commented 7 months ago

Probably related, I think this used to work(or maybe I had just not tested it appropriately), but doesn't appear to work on ansible.utils 3.1.0

task:

- name: debug range
  debug:
    msg: "{{ '10.151.120.0' | ansible.utils.ipaddr('10.152.120.0/22') }}"

Expected:

TASK [hamstring.linux.netchecks : debug range] ************************************************************************************************************
task path: /home/hamstring/.ansible/collections/ansible_collections/hamstring/linux/roles/netchecks/tasks/preflight.yml:9
ok: [testbed.home.arpa] => {
    "msg": "false"
}

Actual:

TASK [hamstring.linux.netchecks : debug range] ************************************************************************************************************
task path: /home/hamstring/.ansible/collections/ansible_collections/hamstring/linux/roles/netchecks/tasks/preflight.yml:9
ok: [testbed.home.arpa] => {
    "msg": ""
}