[stable-2.11] ini lookup - catch and handle duplicate key and missing section errors (#74629) (#74682)

* Remove unused import
* Add integration tests for errors
* Cleanup formatting on existing tests
* Add changelog.
(cherry picked from commit 0affe4d027)

* Add note about Py2/3 differences regarding key names

* Fix tests

Kwarg parsing wasn't backported.

changelogs/fragments/74601-ini-lookup-handle-errors.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - ini lookup - handle errors for duplicate keys and missing sections (https://github.com/ansible/ansible/issues/74601)

lib/ansible/plugins/lookup/ini.py

@@ -15,7 +15,10 @@ DOCUMENTATION = """
       - "You can also read a property file which - in this case - does not contain section."
     options:
       _terms:
-        description: The key(s) to look up
+        description:
+            The key(s) to look up. On Python 2, key names are case B(insensitive).
+            In Python 3, key names are case B(sensitive). Duplicate key names found
+            in a file will result in an error.
         required: True
       type:
         description: Type of the file. 'properties' refers to the Java properties files.
@@ -63,9 +66,9 @@ import os
 import re
 from io import StringIO
 
-from ansible.errors import AnsibleError, AnsibleAssertionError
+from ansible.errors import AnsibleError, AnsibleAssertionError, AnsibleLookupError
 from ansible.module_utils.six.moves import configparser
-from ansible.module_utils._text import to_bytes, to_text
+from ansible.module_utils._text import to_text, to_native
 from ansible.module_utils.common._collections_compat import MutableSequence
 from ansible.plugins.lookup import LookupBase
 
@@ -153,9 +156,15 @@ class LookupModule(LookupBase):
             config.write(contents)
             config.seek(0, os.SEEK_SET)
 
-            self.cp.readfp(config)
-            var = self.get_value(key, paramvals['section'],
-                                 paramvals['default'], paramvals['re'])
+            try:
+                self.cp.readfp(config)
+            except configparser.DuplicateOptionError as doe:
+                raise AnsibleLookupError("Duplicate option in '{file}': {error}".format(file=paramvals['file'], error=to_native(doe)))
+
+            try:
+                var = self.get_value(key, paramvals['section'], paramvals['default'], paramvals['re'])
+            except configparser.NoSectionError:
+                raise AnsibleLookupError("No section '{section}' in {file}".format(section=paramvals['section'], file=paramvals['file']))
             if var is not None:
                 if isinstance(var, MutableSequence):
                     for v in var:

test/integration/targets/lookup_ini/duplicate.ini

@@ -0,0 +1,3 @@
+[reggae]
+name = bob
+name = marley

test/integration/targets/lookup_ini/duplicate_case_check.ini

@@ -0,0 +1,3 @@
+[reggae]
+name = bob
+NAME = marley

test/integration/targets/lookup_ini/inventory

@@ -0,0 +1,2 @@
+[all]
+testhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}"

test/integration/targets/lookup_ini/runme.sh

@@ -2,4 +2,5 @@
 
 set -eux
 
-ansible-playbook test_lookup_properties.yml -i ../../inventory -v "$@"
+ansible-playbook test_lookup_properties.yml -i inventory -v "$@"
+ansible-playbook test_errors.yml -i inventory -v "$@"

test/integration/targets/lookup_ini/test_errors.yml

@@ -0,0 +1,38 @@
+- name: Test INI lookup errors
+  hosts: testhost
+
+  tasks:
+    - name: Test for failure on Python 3
+      when: ansible_facts.python.version_info[0] >= 3
+      block:
+        - name: Lookup a file with duplicate keys
+          debug:
+            msg: "{{ lookup('ini', 'reggae file=duplicate.ini section=reggae') }}"
+          ignore_errors: yes
+          register: duplicate
+
+        - name: Lookup a file with keys that differ only in case
+          debug:
+            msg: "{{ lookup('ini', 'reggae file=duplicate_case_check.ini section=reggae') }}"
+          ignore_errors: yes
+          register: duplicate_case_sensitive
+
+        - name: Ensure duplicate key errers were handled properly
+          assert:
+            that:
+              - duplicate is failed
+              - "'Duplicate option in' in duplicate.msg"
+              - duplicate_case_sensitive is failed
+              - "'Duplicate option in' in duplicate_case_sensitive.msg"
+
+    - name: Lookup a file with a missing section
+      debug:
+        msg: "{{ lookup('ini', 'reggae file=lookup.ini section=missing') }}"
+      ignore_errors: yes
+      register: missing_section
+
+    - name: Ensure error was shown for a missing section
+      assert:
+        that:
+          - missing_section is failed
+          - "'No section' in missing_section.msg"

test/integration/targets/lookup_ini/test_lookup_properties.yml

@@ -1,70 +1,87 @@
----
-- name: "Lookup test"
-  hosts: "localhost"
-#  connection: local
+- name: Lookup test
+  hosts: testhost
+
   tasks:
     - name: "read properties value"
       set_fact:
-        test1:            "{{lookup('ini', 'value1            type=properties  file=lookup.properties')}}"
-        test2:            "{{lookup('ini', 'value2            type=properties  file=lookup.properties')}}"
-        test_dot:         "{{lookup('ini', 'value.dot         type=properties  file=lookup.properties')}}"
+        test1: "{{lookup('ini', 'value1            type=properties  file=lookup.properties')}}"
+        test2: "{{lookup('ini', 'value2 type=properties file=lookup.properties')}}"
+        test_dot: "{{lookup('ini', 'value.dot type=properties file=lookup.properties')}}"
         field_with_space: "{{lookup('ini', 'field.with.space  type=properties  file=lookup.properties')}}"
+
     - assert:
         that: "{{item}} is defined"
       with_items: [ 'test1', 'test2', 'test_dot', 'field_with_space' ]
+
     - name: "read ini value"
       set_fact:
-        value1_global:    "{{lookup('ini', 'value1            section=global   file=lookup.ini')}}"
-        value2_global:    "{{lookup('ini', 'value2            section=global   file=lookup.ini')}}"
-        value1_section1:  "{{lookup('ini', 'value1            section=section1 file=lookup.ini')}}"
-        field_with_unicode: "{{lookup('ini', 'unicode         section=global   file=lookup.ini')}}"
+        value1_global: "{{lookup('ini', 'value1 section=global file=lookup.ini')}}"
+        value2_global: "{{lookup('ini', 'value2 section=global file=lookup.ini')}}"
+        value1_section1: "{{lookup('ini', 'value1 section=section1 file=lookup.ini')}}"
+        field_with_unicode: "{{lookup('ini', 'unicode section=global file=lookup.ini')}}"
+
     - debug: var={{item}}
       with_items: [ 'value1_global', 'value2_global', 'value1_section1', 'field_with_unicode' ]
+
     - assert:
         that:
           - "field_with_unicode == 'été indien où à château français ïîôû'"
+
     - name: "read ini value from iso8859-15 file"
       set_fact:
         field_with_unicode: "{{lookup('ini', 'field_with_unicode section=global encoding=iso8859-1 file=lookup-8859-15.ini')}}"
+
     - assert:
         that:
           - "field_with_unicode == 'été indien où à château français ïîôû'"
+
     - name: "read ini value with section and regexp"
       set_fact:
         value_section:    "{{lookup('ini', 'value[1-2]        section=value_section file=lookup.ini re=true')}}"
         other_section:    "{{lookup('ini', 'other[1-2]        section=other_section file=lookup.ini re=true')}}"
+
     - debug: var={{item}}
       with_items: [ 'value_section', 'other_section' ]
+
     - assert:
         that:
           - "value_section == '1,2'"
           - "other_section == '4,5'"
+
     - name: "Reading unknown value"
       set_fact:
         unknown:  "{{lookup('ini', 'unknown  default=unknown section=section1 file=lookup.ini')}}"
+
     - debug: var=unknown
+
     - assert:
         that:
           - 'unknown == "unknown"'
+
     - name: "Looping over section section1"
       debug: msg="{{item}}"
       with_ini: value[1-2] section=section1      file=lookup.ini re=true
       register: _
+
     - assert:
         that:
           - '_.results.0.item == "section1/value1"'
           - '_.results.1.item == "section1/value2"'
+
     - name: "Looping over section value_section"
       debug: msg="{{item}}"
       with_ini: value[1-2] section=value_section file=lookup.ini re=true
       register: _
+
     - assert:
         that:
           - '_.results.0.item == "1"'
           - '_.results.1.item == "2"'
+
     - debug: msg="{{item}}"
       with_ini: value[1-2] section=section1 file=lookup.ini re=true
       register: _
+
     - assert:
         that:
           - '_.results.0.item == "section1/value1"'