From 0affe4d027ef4ca5517c06da44dcd1b5b8e2544c Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Wed, 12 May 2021 16:57:02 -0400 Subject: [PATCH] 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 --- .../74601-ini-lookup-handle-errors.yml | 2 + lib/ansible/plugins/lookup/ini.py | 15 ++++-- .../targets/lookup_ini/duplicate.ini | 3 ++ .../lookup_ini/duplicate_case_check.ini | 3 ++ test/integration/targets/lookup_ini/inventory | 2 + test/integration/targets/lookup_ini/runme.sh | 3 +- .../targets/lookup_ini/test_errors.yml | 50 ++++++++++++++++++ .../lookup_ini/test_lookup_properties.yml | 51 ++++++++++--------- 8 files changed, 100 insertions(+), 29 deletions(-) create mode 100644 changelogs/fragments/74601-ini-lookup-handle-errors.yml create mode 100644 test/integration/targets/lookup_ini/duplicate.ini create mode 100644 test/integration/targets/lookup_ini/duplicate_case_check.ini create mode 100644 test/integration/targets/lookup_ini/inventory create mode 100644 test/integration/targets/lookup_ini/test_errors.yml diff --git a/changelogs/fragments/74601-ini-lookup-handle-errors.yml b/changelogs/fragments/74601-ini-lookup-handle-errors.yml new file mode 100644 index 00000000000..d15df078e9e --- /dev/null +++ b/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) diff --git a/lib/ansible/plugins/lookup/ini.py b/lib/ansible/plugins/lookup/ini.py index b8ba792ef11..eff6413dead 100644 --- a/lib/ansible/plugins/lookup/ini.py +++ b/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: diff --git a/test/integration/targets/lookup_ini/duplicate.ini b/test/integration/targets/lookup_ini/duplicate.ini new file mode 100644 index 00000000000..db510ddff04 --- /dev/null +++ b/test/integration/targets/lookup_ini/duplicate.ini @@ -0,0 +1,3 @@ +[reggae] +name = bob +name = marley diff --git a/test/integration/targets/lookup_ini/duplicate_case_check.ini b/test/integration/targets/lookup_ini/duplicate_case_check.ini new file mode 100644 index 00000000000..abb0128b46e --- /dev/null +++ b/test/integration/targets/lookup_ini/duplicate_case_check.ini @@ -0,0 +1,3 @@ +[reggae] +name = bob +NAME = marley diff --git a/test/integration/targets/lookup_ini/inventory b/test/integration/targets/lookup_ini/inventory new file mode 100644 index 00000000000..ae7642708cf --- /dev/null +++ b/test/integration/targets/lookup_ini/inventory @@ -0,0 +1,2 @@ +[all] +testhost ansible_connection=local ansible_python_interpreter="{{ ansible_playbook_python }}" diff --git a/test/integration/targets/lookup_ini/runme.sh b/test/integration/targets/lookup_ini/runme.sh index 71a507de426..76f836a9b96 100755 --- a/test/integration/targets/lookup_ini/runme.sh +++ b/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 "$@" diff --git a/test/integration/targets/lookup_ini/test_errors.yml b/test/integration/targets/lookup_ini/test_errors.yml new file mode 100644 index 00000000000..07962202301 --- /dev/null +++ b/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' diff --git a/test/integration/targets/lookup_ini/test_lookup_properties.yml b/test/integration/targets/lookup_ini/test_lookup_properties.yml index 915c68eeb6e..a6fc0f7d7c2 100644 --- a/test/integration/targets/lookup_ini/test_lookup_properties.yml +++ b/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'