From 1b4fd23ba6dab52a395278e3ef7ca994e0819d60 Mon Sep 17 00:00:00 2001 From: Rick Elrod Date: Fri, 10 Jul 2020 16:21:03 -0500 Subject: [PATCH] csvfile: use parse_kv() for args, add tests (#70550) Change: - Use parse_kv() for parsing in the csvfile lookup plugin. This allows us to handle multi-word search keys and filenames. Previously, the plugin split on space and so none of these things worked as expected. - Add integration tests for csvfile, testing a plethora of weird cases. Test Plan: - New integration tests, CI Tickets: - Fixes #70545 Signed-off-by: Rick Elrod --- changelogs/fragments/csvfile-parse_kv.yml | 3 ++ lib/ansible/plugins/lookup/csvfile.py | 25 ++++++--- .../targets/lookup_csvfile/aliases | 2 + .../files/cool list of things.csv | 3 ++ .../targets/lookup_csvfile/files/crlf.csv | 2 + .../targets/lookup_csvfile/files/people.csv | 6 +++ .../targets/lookup_csvfile/files/tabs.csv | 4 ++ .../targets/lookup_csvfile/files/x1a.csv | 3 ++ .../targets/lookup_csvfile/tasks/main.yml | 54 +++++++++++++++++++ test/sanity/ignore.txt | 1 + 10 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/csvfile-parse_kv.yml create mode 100644 test/integration/targets/lookup_csvfile/aliases create mode 100644 test/integration/targets/lookup_csvfile/files/cool list of things.csv create mode 100644 test/integration/targets/lookup_csvfile/files/crlf.csv create mode 100644 test/integration/targets/lookup_csvfile/files/people.csv create mode 100644 test/integration/targets/lookup_csvfile/files/tabs.csv create mode 100644 test/integration/targets/lookup_csvfile/files/x1a.csv create mode 100644 test/integration/targets/lookup_csvfile/tasks/main.yml diff --git a/changelogs/fragments/csvfile-parse_kv.yml b/changelogs/fragments/csvfile-parse_kv.yml new file mode 100644 index 00000000000..a2c85824148 --- /dev/null +++ b/changelogs/fragments/csvfile-parse_kv.yml @@ -0,0 +1,3 @@ +minor_changes: + - The ``csvfile`` lookup plugin now uses ``parse_kv()`` internally. As a result, multi-word search keys can now be passed. + - The ``csvfile`` lookup plugin's documentation has been fixed; it erroneously said that the delimiter could be ``t`` which was never true. We now accept ``\t``, however, and the error in the documentation has been fixed to note that. diff --git a/lib/ansible/plugins/lookup/csvfile.py b/lib/ansible/plugins/lookup/csvfile.py index 03d9e6e9f51..e0ffa69be94 100644 --- a/lib/ansible/plugins/lookup/csvfile.py +++ b/lib/ansible/plugins/lookup/csvfile.py @@ -11,16 +11,17 @@ DOCUMENTATION = """ short_description: read data from a TSV or CSV file description: - The csvfile lookup reads the contents of a file in CSV (comma-separated value) format. - The lookup looks for the row where the first column matches keyname, and returns the value in the second column, unless a different column is specified. + The lookup looks for the row where the first column matches keyname (which can be multiple words) + and returns the value in the C(col) column (default 1, which indexed from 0 means the second column in the file). options: col: - description: column to return (0 index). + description: column to return (0 indexed). default: "1" default: description: what to return if the value is not found in the file. default: '' delimiter: - description: field separator in the file, for a tab you can specify "TAB" or "t". + description: field separator in the file, for a tab you can specify C(TAB) or C(\\t). default: TAB file: description: name of the CSV/TSV file to open. @@ -31,6 +32,10 @@ DOCUMENTATION = """ version_added: "2.1" notes: - The default is for TSV files (tab delimited) not CSV (comma delimited) ... yes the name is misleading. + - As of version 2.11, the search parameter (text that must match the first column of the file) and filename parameter can be multi-word. + - For historical reasons, in the search keyname, quotes are treated + literally and cannot be used around the string unless they appear + (escaped as required) in the first column of the file you are parsing. """ EXAMPLES = """ @@ -62,6 +67,7 @@ import codecs import csv from ansible.errors import AnsibleError, AnsibleAssertionError +from ansible.parsing.splitter import parse_kv from ansible.plugins.lookup import LookupBase from ansible.module_utils.six import PY2 from ansible.module_utils._text import to_bytes, to_native, to_text @@ -129,8 +135,12 @@ class LookupModule(LookupBase): ret = [] for term in terms: - params = term.split() - key = params[0] + kv = parse_kv(term) + + if '_raw_params' not in kv: + raise AnsibleError('Search key is required but was not found') + + key = kv['_raw_params'] paramvals = { 'col': "1", # column to return @@ -142,8 +152,9 @@ class LookupModule(LookupBase): # parameters specified? try: - for param in params[1:]: - name, value = param.split('=') + for name, value in kv.items(): + if name == '_raw_params': + continue if name not in paramvals: raise AnsibleAssertionError('%s not in paramvals' % name) paramvals[name] = value diff --git a/test/integration/targets/lookup_csvfile/aliases b/test/integration/targets/lookup_csvfile/aliases new file mode 100644 index 00000000000..45489be80c6 --- /dev/null +++ b/test/integration/targets/lookup_csvfile/aliases @@ -0,0 +1,2 @@ +shippable/posix/group2 +skip/python2.6 # lookups are controller only, and we no longer support Python 2.6 on the controller diff --git a/test/integration/targets/lookup_csvfile/files/cool list of things.csv b/test/integration/targets/lookup_csvfile/files/cool list of things.csv new file mode 100644 index 00000000000..b1a74a0abef --- /dev/null +++ b/test/integration/targets/lookup_csvfile/files/cool list of things.csv @@ -0,0 +1,3 @@ +woo,i,have,spaces,in,my,filename +i,am,so,cool,haha,be,jealous +maybe,i,will,work,like,i,should diff --git a/test/integration/targets/lookup_csvfile/files/crlf.csv b/test/integration/targets/lookup_csvfile/files/crlf.csv new file mode 100644 index 00000000000..a17f6c47579 --- /dev/null +++ b/test/integration/targets/lookup_csvfile/files/crlf.csv @@ -0,0 +1,2 @@ +this file,has,crlf,line,endings +ansible,parses,them,just,fine diff --git a/test/integration/targets/lookup_csvfile/files/people.csv b/test/integration/targets/lookup_csvfile/files/people.csv new file mode 100644 index 00000000000..f93498cf22b --- /dev/null +++ b/test/integration/targets/lookup_csvfile/files/people.csv @@ -0,0 +1,6 @@ +# Last,First,Email,Extension +Smith,Jane,jsmith@example.com,1234 +Ipsum,Lorem,lipsum@another.example.com,9001 +"German von Lastname",Demo,hello@example.com,123123 +Example,Person,"crazy email"@example.com,9876 +"""The Rock"" Johnson",Dwayne,uhoh@example.com,1337 diff --git a/test/integration/targets/lookup_csvfile/files/tabs.csv b/test/integration/targets/lookup_csvfile/files/tabs.csv new file mode 100644 index 00000000000..69f4d876de2 --- /dev/null +++ b/test/integration/targets/lookup_csvfile/files/tabs.csv @@ -0,0 +1,4 @@ +fruit bananas 30 +fruit apples 9 +electronics tvs 8 +shoes sneakers 26 diff --git a/test/integration/targets/lookup_csvfile/files/x1a.csv b/test/integration/targets/lookup_csvfile/files/x1a.csv new file mode 100644 index 00000000000..d2d5a0d4592 --- /dev/null +++ b/test/integration/targets/lookup_csvfile/files/x1a.csv @@ -0,0 +1,3 @@ +separatedbyx1achars +againbecause +wecan diff --git a/test/integration/targets/lookup_csvfile/tasks/main.yml b/test/integration/targets/lookup_csvfile/tasks/main.yml new file mode 100644 index 00000000000..8e65b2e245c --- /dev/null +++ b/test/integration/targets/lookup_csvfile/tasks/main.yml @@ -0,0 +1,54 @@ +- set_fact: + this_will_error: "{{ lookup('csvfile', 'file=people.csv delimiter=, col=1') }}" + ignore_errors: yes + register: no_keyword + +- name: Make sure we failed above + assert: + that: + - no_keyword is failed + - > + "Search key is required but was not found" in no_keyword.msg + +- name: Check basic comma-separated file + assert: + that: + - lookup('csvfile', 'Smith file=people.csv delimiter=, col=1') == "Jane" + - lookup('csvfile', 'German von Lastname file=people.csv delimiter=, col=1') == "Demo" + +- name: Check tab-separated file + assert: + that: + - lookup('csvfile', 'electronics file=tabs.csv delimiter=TAB col=1') == "tvs" + - lookup('csvfile', 'fruit file=tabs.csv delimiter=TAB col=1') == "bananas" + - lookup('csvfile', 'fruit file=tabs.csv delimiter="\t" col=1') == "bananas" + +- name: Check \x1a-separated file + assert: + that: + - lookup('csvfile', 'again file=x1a.csv delimiter=\x1a col=1') == "because" + +- name: Check CSV file with CRLF line endings + assert: + that: + - lookup('csvfile', 'this file file=crlf.csv delimiter=, col=2') == "crlf" + - lookup('csvfile', 'ansible file=crlf.csv delimiter=, col=1') == "parses" + +- name: Check file with multi word filename + assert: + that: + - lookup('csvfile', 'maybe file="cool list of things.csv" delimiter=, col=3') == "work" + +- name: Test default behavior + assert: + that: + - lookup('csvfile', 'notfound file=people.csv delimiter=, col=2') == [] + - lookup('csvfile', 'notfound file=people.csv delimiter=, col=2, default=what?') == "what?" + +# NOTE: For historical reasons, this is correct; quotes in the search field must +# be treated literally as if they appear (escaped as required) in the field in the +# file. They cannot be used to surround the search text in general. +- name: Test quotes in the search field + assert: + that: + - lookup('csvfile', '"The Rock" Johnson file=people.csv delimiter=, col=1') == "Dwayne" diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 18cf80e6bd1..dabb59b66cb 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -203,6 +203,7 @@ test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.1/DSCResources/AN test/integration/targets/incidental_win_dsc/files/xTestDsc/1.0.1/xTestDsc.psd1 pslint!skip test/integration/targets/incidental_win_ping/library/win_ping_syntax_error.ps1 pslint!skip test/integration/targets/incidental_win_reboot/templates/post_reboot.ps1 pslint!skip +test/integration/targets/lookup_csvfile/files/crlf.csv line-endings test/integration/targets/lookup_ini/lookup-8859-15.ini no-smart-quotes test/integration/targets/module_precedence/lib_with_extension/a.ini shebang test/integration/targets/module_precedence/lib_with_extension/ping.ini shebang