From 8b2dd5fdd3cf13dbfde082c730542537be76d9f6 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Tue, 23 Jan 2024 14:48:32 -0500 Subject: [PATCH] updated looups to use set_option in custom parse (#82425) limit password params updated test error message catching make sure we reset params for each term ensure we only update options if we have em --- changelogs/fragments/lookups_updated.yml | 2 + lib/ansible/plugins/lookup/csvfile.py | 7 +- lib/ansible/plugins/lookup/ini.py | 6 +- lib/ansible/plugins/lookup/password.py | 29 ++++---- lib/ansible/plugins/lookup/sequence.py | 70 ++++++------------- .../targets/lookup_sequence/tasks/main.yml | 8 +-- 6 files changed, 53 insertions(+), 69 deletions(-) create mode 100644 changelogs/fragments/lookups_updated.yml diff --git a/changelogs/fragments/lookups_updated.yml b/changelogs/fragments/lookups_updated.yml new file mode 100644 index 00000000000..15104bc23d8 --- /dev/null +++ b/changelogs/fragments/lookups_updated.yml @@ -0,0 +1,2 @@ +bugfixes: + - All core lookups now use set_option(s) even when doing their own custom parsing. This ensures that the options are always the proper type. diff --git a/lib/ansible/plugins/lookup/csvfile.py b/lib/ansible/plugins/lookup/csvfile.py index 778138847ae..9d199d82190 100644 --- a/lib/ansible/plugins/lookup/csvfile.py +++ b/lib/ansible/plugins/lookup/csvfile.py @@ -160,6 +160,7 @@ class LookupModule(LookupBase): # parameters override per term using k/v try: + reset_params = False for name, value in kv.items(): if name == '_raw_params': continue @@ -167,7 +168,11 @@ class LookupModule(LookupBase): raise AnsibleAssertionError('%s is not a valid option' % name) self._deprecate_inline_kv() - paramvals[name] = value + self.set_option(name, value) + reset_params = True + + if reset_params: + paramvals = self.get_options() except (ValueError, AssertionError) as e: raise AnsibleError(e) diff --git a/lib/ansible/plugins/lookup/ini.py b/lib/ansible/plugins/lookup/ini.py index 225e7d8e8fa..cdc9a1540cd 100644 --- a/lib/ansible/plugins/lookup/ini.py +++ b/lib/ansible/plugins/lookup/ini.py @@ -154,16 +154,20 @@ class LookupModule(LookupBase): params = _parse_params(term, paramvals) try: updated_key = False + updated_options = False for param in params: if '=' in param: name, value = param.split('=') if name not in paramvals: raise AnsibleLookupError('%s is not a valid option.' % name) - paramvals[name] = value + self.set_option(name, value) + updated_options = True elif key == term: # only take first, this format never supported multiple keys inline key = param updated_key = True + if updated_options: + paramvals = self.get_options() except ValueError as e: # bad params passed raise AnsibleLookupError("Could not use '%s' from '%s': %s" % (param, params, to_native(e)), orig_exc=e) diff --git a/lib/ansible/plugins/lookup/password.py b/lib/ansible/plugins/lookup/password.py index 4db89d8c025..84894e21e9a 100644 --- a/lib/ansible/plugins/lookup/password.py +++ b/lib/ansible/plugins/lookup/password.py @@ -331,29 +331,32 @@ class LookupModule(LookupBase): if invalid_params: raise AnsibleError('Unrecognized parameter(s) given to password lookup: %s' % ', '.join(invalid_params)) - # Set defaults - params['length'] = int(params.get('length', self.get_option('length'))) - params['encrypt'] = params.get('encrypt', self.get_option('encrypt')) - params['ident'] = params.get('ident', self.get_option('ident')) - params['seed'] = params.get('seed', self.get_option('seed')) - - params['chars'] = params.get('chars', self.get_option('chars')) - if params['chars'] and isinstance(params['chars'], string_types): + # update options with what we got + if params: + self.set_options(direct=params) + + # chars still might need more + chars = params.get('chars', self.get_option('chars')) + if chars and isinstance(chars, string_types): tmp_chars = [] - if u',,' in params['chars']: + if u',,' in chars: tmp_chars.append(u',') - tmp_chars.extend(c for c in params['chars'].replace(u',,', u',').split(u',') if c) - params['chars'] = tmp_chars + tmp_chars.extend(c for c in chars.replace(u',,', u',').split(u',') if c) + self.set_option('chars', tmp_chars) + + # return processed params + for field in VALID_PARAMS: + params[field] = self.get_option(field) return relpath, params def run(self, terms, variables, **kwargs): ret = [] - self.set_options(var_options=variables, direct=kwargs) - for term in terms: + self.set_options(var_options=variables, direct=kwargs) + changed = None relpath, params = self._parse_parameters(term) path = self._loader.path_dwim(relpath) diff --git a/lib/ansible/plugins/lookup/sequence.py b/lib/ansible/plugins/lookup/sequence.py index 4954accfe42..9efe7cef53a 100644 --- a/lib/ansible/plugins/lookup/sequence.py +++ b/lib/ansible/plugins/lookup/sequence.py @@ -19,21 +19,21 @@ DOCUMENTATION = """ options: start: description: number at which to start the sequence - default: 0 + default: 1 type: integer end: description: number at which to end the sequence, dont use this with count type: integer - default: 0 count: description: number of elements in the sequence, this is not to be used with end type: integer - default: 0 stride: description: increments between sequence numbers, the default is 1 unless the end is less than the start, then it is -1. type: integer + default: 1 format: description: return a string with the generated number formatted in + default: "%d" """ EXAMPLES = """ @@ -97,6 +97,7 @@ SHORTCUT = re_compile( "(:(.+))?$", # Group 5, Group 6: Format String IGNORECASE ) +FIELDS = frozenset(('start', 'end', 'stride', 'count', 'format')) class LookupModule(LookupBase): @@ -138,30 +139,12 @@ class LookupModule(LookupBase): calculating the number of entries in a sequence when a stride is specified. """ - def reset(self): - """set sensible defaults""" - self.start = 1 - self.count = None - self.end = None - self.stride = 1 - self.format = "%d" - def parse_kv_args(self, args): """parse key-value style arguments""" - for arg in ["start", "end", "count", "stride"]: - try: - arg_raw = args.pop(arg, None) - if arg_raw is None: - continue - arg_cooked = int(arg_raw, 0) - setattr(self, arg, arg_cooked) - except ValueError: - raise AnsibleError( - "can't parse %s=%s as integer" - % (arg, arg_raw) - ) - if 'format' in args: - self.format = args.pop("format") + for arg in FIELDS: + value = args.pop(arg, None) + if value is not None: + self.set_option(arg, value) if args: raise AnsibleError( "unrecognized arguments to with_sequence: %s" @@ -176,33 +159,17 @@ class LookupModule(LookupBase): dummy, start, end, dummy, stride, dummy, format = match.groups() - if start is not None: - try: - start = int(start, 0) - except ValueError: - raise AnsibleError("can't parse start=%s as integer" % start) - if end is not None: - try: - end = int(end, 0) - except ValueError: - raise AnsibleError("can't parse end=%s as integer" % end) - if stride is not None: - try: - stride = int(stride, 0) - except ValueError: - raise AnsibleError("can't parse stride=%s as integer" % stride) - - if start is not None: - self.start = start - if end is not None: - self.end = end - if stride is not None: - self.stride = stride - if format is not None: - self.format = format + for key in FIELDS: + value = locals().get(key, None) + if value is not None: + self.set_option(key, value) return True + def set_fields(self): + for f in FIELDS: + setattr(self, f, self.get_option(f)) + def sanity_check(self): if self.count is None and self.end is None: raise AnsibleError("must specify count or end in with_sequence") @@ -245,7 +212,8 @@ class LookupModule(LookupBase): for term in terms: try: - self.reset() # clear out things for this iteration + # set defaults/global + self.set_options(direct=kwargs) try: if not self.parse_simple_args(term): self.parse_kv_args(parse_kv(term)) @@ -254,7 +222,9 @@ class LookupModule(LookupBase): except Exception as e: raise AnsibleError("unknown error parsing with_sequence arguments: %r. Error was: %s" % (term, e)) + self.set_fields() self.sanity_check() + if self.stride != 0: results.extend(self.generate_sequence()) except AnsibleError: diff --git a/test/integration/targets/lookup_sequence/tasks/main.yml b/test/integration/targets/lookup_sequence/tasks/main.yml index e64801d3c6d..3d74339e8cd 100644 --- a/test/integration/targets/lookup_sequence/tasks/main.yml +++ b/test/integration/targets/lookup_sequence/tasks/main.yml @@ -89,7 +89,7 @@ - assert: that: - ansible_failed_task.name == "EXPECTED FAILURE - test bad kv value" - - ansible_failed_result.msg == "can't parse start=A as integer" + - ansible_failed_result.msg.startswith("Invalid type for") - block: - name: EXPECTED FAILURE - test bad simple form start value @@ -102,7 +102,7 @@ - assert: that: - ansible_failed_task.name == "EXPECTED FAILURE - test bad simple form start value" - - ansible_failed_result.msg == "can't parse start=A as integer" + - ansible_failed_result.msg.startswith("Invalid type for") - block: - name: EXPECTED FAILURE - test bad simple form end value @@ -115,7 +115,7 @@ - assert: that: - ansible_failed_task.name == "EXPECTED FAILURE - test bad simple form end value" - - ansible_failed_result.msg == "can't parse end=B as integer" + - ansible_failed_result.msg.startswith("Invalid type for") - block: - name: EXPECTED FAILURE - test bad simple form stride value @@ -128,7 +128,7 @@ - assert: that: - ansible_failed_task.name == "EXPECTED FAILURE - test bad simple form stride value" - - ansible_failed_result.msg == "can't parse stride=C as integer" + - ansible_failed_result.msg.startswith("Invalid type for") - block: - name: EXPECTED FAILURE - test no count or end