dig + wantlist can return a list with an empty string instead of an empty list

[RomainMou]

Summary

When I use the lookup plugin community.general.dig with wantlist=true and there is no record, the plugin return [""] instead of [].
community.general.dnstxt has the same issue.

Issue Type

Bug Report

Component Name

dig

Ansible Version

ansible [core 2.13.5]
  config file = /home/user/Documents/dev/deployment/ansible.cfg
  configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/user/.virtualenvs/tmp-9bc177cc8a2084d/lib/python3.10/site-packages/ansible
  ansible collection location = /home/user/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/user/.virtualenvs/tmp-9bc177cc8a2084d/bin/ansible
  python version = 3.10.8 (main, Oct 13 2022, 21:13:48) [GCC 12.2.0]
  jinja version = 3.1.2
  libyaml = True

Community.general Version

# /home/user/.virtualenvs/tmp-9bc177cc8a2084d/lib/python3.10/site-packages/ansible_collections
Collection        Version
----------------- -------
community.general 5.7.0  

# /home/user/.ansible/collections/ansible_collections
Collection        Version
----------------- -------
community.general 5.8.0

Configuration

DEFAULT_FILTER_PLUGIN_PATH(/home/user/Documents/dev/deployment/ansible.cfg) = ['/home/user/Documents/dev/deployment/ansible_plugins/filter_plugins']
DEFAULT_TIMEOUT(/home/user/Documents/dev/deployment/ansible.cfg) = 60
DEFAULT_VAULT_PASSWORD_FILE(/home/user/Documents/dev/deployment/ansible.cfg) = /home/user/Documents/dev/deployment/.get-vault-pass.sh

OS / Environment

No response

Steps to Reproduce

- name: "Test lookup"
  hosts: localhost
  gather_facts: false
  tasks:
    - name: "Check version of community.general"
      ansible.builtin.debug:
        msg: "community.general version {{ lookup('community.general.collection_version', 'community.general') }}"

    - name: "There is no NS, so we expect an empty list"
      ansible.builtin.set_fact:
        ns: "{{ lookup('community.general.dig',
                'www.google.com',
                'qtype=NS',
                wantlist=true) }}"

    - name: "Print retrieve NS"
      ansible.builtin.debug:
        var: ns

Expected Results

TASK [Print retrieve NS] ****************************************************************************************************************************************************************************
ok: [localhost] => {
"ns": []
}

Actual Results

PLAY [Test lookup] **********************************************************************************************************************************************************************************

TASK [Check version of community.general] ***********************************************************************************************************************************************************
ok: [localhost] => {
    "msg": "community.general version 5.8.0"
}

TASK [There is NS, so we expect empty result] *******************************************************************************************************************************************************
ok: [localhost]

TASK [Print retrieve NS] ****************************************************************************************************************************************************************************
ok: [localhost] => {
    "ns": [
        ""
    ]
}

PLAY RECAP ******************************************************************************************************************************************************************************************
localhost                  : ok=3    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/lookup/dig.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @jpmens
click here for bot help

[felixfontein]

This is "as designed" (https://github.com/ansible-collections/community.general/blob/main/plugins/lookup/dig.py#L382), and I don't see a place in the documentation which says that this won't be the case, so I guess it's not a bug. It is definitely nothing I would expect as a user, I fully agree on that.

(I would guess this is a leftover from the times when lookup(..., wantlist=true) and query() did not exist, and Ansible always converted an empty list as the return value to an empty string.)

[jpmens]

I do not recall why that's there or even if I originally put that there so I tend to agree with @felixfontein that it's not a bug...

@RomainMou what happens when you use query() instead of lookup()? Do you then get an empty array? If so, I should think that closes the case, otherwise I think we ought to fix this.

[RomainMou]

I have the same result with query:

    - name: "There is no NS, so we expect an empty list"
      ansible.builtin.set_fact:
        ns: "{{ query('community.general.dig',
                'www.google.com',
                'qtype=NS') }}"

TASK [Print retrieve NS with query] ************************************************************
ok: [localhost] => {
   "ns": [
       ""
   ]
}

[jpmens]

Thank you. I consider this a bug, and if @felixfontein agrees I will attempt to address this and fix.

[felixfontein]

I'm a bit worried about this, this behavior has been the case for many years now, and I'm very sure there are hundreds of playbooks and roles that assume that the lookup will return [""] if no record is found, and which potentially no longer work when this is changed. This behavior has been there since at least ansible/ansible@74079db, so for over seven years (!).

I think the only clean way to fix this is to 1) add a new option which controls this behavior with default = current behavior, 2) deprecate the current default, 3) once the deprecation expires (should be in a long enough time in the future) switch the default. This is annoying and quite some extra effort, but just changing the behavior will likely break too many playbooks and roles.

[jpmens]

Hundreds of playbooks that use the dig lookup? And why aren’t we rich and famous yet?!? ;)

[RomainMou]

As stated in Ansible documentation about collection versioning:

Collections MUST adhere to semantic versioning.

You can fix bugs in patch releases but not add new features or deprecate things.
You can add new features and deprecate things in minor releases, but not remove things or change behavior of existing features.
You can only remove things or make breaking changes in major releases.

https://docs.ansible.com/ansible/devel/community/collection_contributors/collection_releasing.html#collection-versioning-and-deprecation

This modification could broke some workflows. So I guess @felixfontein is right about a depreciation warning until the next major version of the collection. But I think it should be eventually fixed. I only tested dig and dnstxt, but if there is a long depreciation warning maybe it could be the occasion to check the others, maybe there is some standardization to do.

[felixfontein]

@RomainMou also see the paragraph on "Deprecation" in #582. We'd have to wait more than for the next major release with changing the default.

[felixfontein]

Hundreds of playbooks that use the dig lookup? And why aren’t we rich and famous yet?!? ;)

Because the plugin doesn't print its authors list every time it's used ;-)

[jpmens]

How do you gentlemen like this, using a boolean flag real_empty= ?

TASK [There is no NS, so we expect an empty list] ***********************************
ok: [localhost]

TASK [Print retrieve NS] ************************************************************
ok: [localhost] => {
    "ns": []
}

TASK [nxdomain (real_empty==False)] *************************************************
ok: [localhost] => {
    "msg": [
        "NXDOMAIN"
    ]
}

TASK [nxdomain (real_empty==True)] **************************************************
ok: [localhost] => {
    "msg": []
}

[jpmens]

diff --git plugins/lookup/dig.py plugins/lookup/dig.py
index d7a7d8ec..847c2c77 100644
--- plugins/lookup/dig.py
+++ plugins/lookup/dig.py
@@ -52,6 +52,13 @@ DOCUMENTATION = '''
         default: false
         type: bool
         version_added: 5.4.0
+      real_empty:
+        description:
+          - Return empty result without empty strings
+          - The default for this option will likely change to C(true) in the future.
+        default: false
+        type: bool
+        version_added: 4.8.1
     notes:
       - ALL is not a record per-se, merely the listed fields are available for any record results you retrieve in the form of a dictionary.
       - While the 'dig' lookup plugin supports anything which dnspython supports out of the box, only a subset can be converted into a dictionary.
@@ -285,6 +292,7 @@ class LookupModule(LookupBase):
         qtype = 'A'
         flat = True
         fail_on_error = False
+        real_empty = False
         rdclass = dns.rdataclass.from_text('IN')
 
         for t in terms:
@@ -325,6 +333,8 @@ class LookupModule(LookupBase):
                     myres.retry_servfail = boolean(arg)
                 elif opt == 'fail_on_error':
                     fail_on_error = boolean(arg)
+                elif opt == 'real_empty':
+                    real_empty = boolean(arg)
 
                 continue
 
@@ -375,15 +385,18 @@ class LookupModule(LookupBase):
         except dns.resolver.NXDOMAIN as err:
             if fail_on_error:
                 raise AnsibleError("Lookup failed: %s" % str(err))
-            ret.append('NXDOMAIN')
+            if not real_empty:
+                ret.append('NXDOMAIN')
         except dns.resolver.NoAnswer as err:
             if fail_on_error:
                 raise AnsibleError("Lookup failed: %s" % str(err))
-            ret.append("")
+            if not real_empty:
+                ret.append("")
         except dns.resolver.Timeout as err:
             if fail_on_error:
                 raise AnsibleError("Lookup failed: %s" % str(err))
-            ret.append('')
+            if not real_empty:
+                ret.append("")
         except dns.exception.DNSException as err:
             raise AnsibleError("dns.resolver unhandled exception %s" % to_native(err))

[felixfontein]

@jpmens looks good. The description needs to be updated to also mention that it will return an empty list instead of NXDOMAIN as well (it just talks about empty strings), and the version_added value would be 6.0.0.

[jpmens]

Thank you for your guidance, Felix. Will prepare PR.

[felixfontein]

#5457 implements the same for dnstxt.