ini lookup - catch and handle duplicate key and missing section errors (#74629)

* Remove unused import
* Add integration tests for errors
* Cleanup formatting on existing tests
* Add changelog

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,7 @@ 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.
         required: True
       type:
         description: Type of the file. 'properties' refers to the Java properties files.
@@ -67,7 +67,7 @@ from collections import defaultdict
 
 from ansible.errors import AnsibleLookupError, AnsibleOptionsError
 from ansible.module_utils.six.moves import configparser
-from ansible.module_utils._text import to_bytes, to_text, to_native
+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
 
@@ -167,8 +167,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,50 @@
+- 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"
+
+    - name: Mix options type and push key out of order
+      debug:
+        msg: "{{ lookup('ini', 'file=lookup.ini', 'value1', section='value_section') }}"
+      register: bad_mojo
+      ignore_errors: yes
+
+    - name: Verify bad behavior reported an error
+      assert:
+        that:
+        - bad_mojo is failed
+        - '"No key to lookup was provided as first term with in string inline option" in bad_mojo.msg'

test/integration/targets/lookup_ini/test_lookup_properties.yml

@@ -1,85 +1,88 @@
----
-- 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')}}"
+        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"'
           - '_.results.1.item == "section1/value2"'
-
-
-    - name: capture bad behaviour
-      block:
-        - name: mix options type and push key out of order
-          debug: msg="{{ lookup('ini', 'file=lookup.ini', 'value1', section='value_section') }}"
-          register: bad_mojo
-          ignore_errors: true
-
-        - name: verify
-          assert:
-            that:
-            - bad_mojo is failed
-            - '"No key to lookup was provided as first term with in string inline option" in bad_mojo.msg'