mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
Properly handle user selection of None
as vars_files (#31313)
* Properly handle user selection of `None` as vars_files In a playbook, if a user has a playbook like: ``` - hosts: localhost connection: local vars_files: tasks: - .... ``` Then `vars_files` will be none, and cause a `TypeError` in vars-manager when it tries to iterate over them. To avoid this, I changed the getter to either send back the vars files from the user, or an empty list when the user passed `None`. * Only replace None with an empty list, not all falsey values * Catch error when vars_files isn't iterable * Move whole `for` loop into try/except and catch TypeError * Line length
This commit is contained in:
parent
ef72eda172
commit
958ad7726a
2 changed files with 50 additions and 43 deletions
|
@ -277,6 +277,8 @@ class Play(Base, Taggable, Become):
|
|||
return self.vars.copy()
|
||||
|
||||
def get_vars_files(self):
|
||||
if self.vars_files is None:
|
||||
return []
|
||||
return self.vars_files
|
||||
|
||||
def get_handlers(self):
|
||||
|
|
|
@ -346,53 +346,58 @@ class VariableManager:
|
|||
if play:
|
||||
all_vars = combine_vars(all_vars, play.get_vars())
|
||||
|
||||
for vars_file_item in play.get_vars_files():
|
||||
# create a set of temporary vars here, which incorporate the extra
|
||||
# and magic vars so we can properly template the vars_files entries
|
||||
temp_vars = combine_vars(all_vars, self._extra_vars)
|
||||
temp_vars = combine_vars(temp_vars, magic_variables)
|
||||
templar = Templar(loader=self._loader, variables=temp_vars)
|
||||
vars_files = play.get_vars_files()
|
||||
try:
|
||||
for vars_file_item in vars_files:
|
||||
# create a set of temporary vars here, which incorporate the extra
|
||||
# and magic vars so we can properly template the vars_files entries
|
||||
temp_vars = combine_vars(all_vars, self._extra_vars)
|
||||
temp_vars = combine_vars(temp_vars, magic_variables)
|
||||
templar = Templar(loader=self._loader, variables=temp_vars)
|
||||
|
||||
# we assume each item in the list is itself a list, as we
|
||||
# support "conditional includes" for vars_files, which mimics
|
||||
# the with_first_found mechanism.
|
||||
vars_file_list = vars_file_item
|
||||
if not isinstance(vars_file_list, list):
|
||||
vars_file_list = [vars_file_list]
|
||||
# we assume each item in the list is itself a list, as we
|
||||
# support "conditional includes" for vars_files, which mimics
|
||||
# the with_first_found mechanism.
|
||||
vars_file_list = vars_file_item
|
||||
if not isinstance(vars_file_list, list):
|
||||
vars_file_list = [vars_file_list]
|
||||
|
||||
# now we iterate through the (potential) files, and break out
|
||||
# as soon as we read one from the list. If none are found, we
|
||||
# raise an error, which is silently ignored at this point.
|
||||
try:
|
||||
for vars_file in vars_file_list:
|
||||
vars_file = templar.template(vars_file)
|
||||
try:
|
||||
data = preprocess_vars(self._loader.load_from_file(vars_file, unsafe=True))
|
||||
if data is not None:
|
||||
for item in data:
|
||||
all_vars = combine_vars(all_vars, item)
|
||||
break
|
||||
except AnsibleFileNotFound:
|
||||
# we continue on loader failures
|
||||
# now we iterate through the (potential) files, and break out
|
||||
# as soon as we read one from the list. If none are found, we
|
||||
# raise an error, which is silently ignored at this point.
|
||||
try:
|
||||
for vars_file in vars_file_list:
|
||||
vars_file = templar.template(vars_file)
|
||||
try:
|
||||
data = preprocess_vars(self._loader.load_from_file(vars_file, unsafe=True))
|
||||
if data is not None:
|
||||
for item in data:
|
||||
all_vars = combine_vars(all_vars, item)
|
||||
break
|
||||
except AnsibleFileNotFound:
|
||||
# we continue on loader failures
|
||||
continue
|
||||
except AnsibleParserError:
|
||||
raise
|
||||
else:
|
||||
# if include_delegate_to is set to False, we ignore the missing
|
||||
# vars file here because we're working on a delegated host
|
||||
if include_delegate_to:
|
||||
raise AnsibleFileNotFound("vars file %s was not found" % vars_file_item)
|
||||
except (UndefinedError, AnsibleUndefinedVariable):
|
||||
if host is not None and self._fact_cache.get(host.name, dict()).get('module_setup') and task is not None:
|
||||
raise AnsibleUndefinedVariable("an undefined variable was found when attempting to template the vars_files item '%s'"
|
||||
% vars_file_item, obj=vars_file_item)
|
||||
else:
|
||||
# we do not have a full context here, and the missing variable could be because of that
|
||||
# so just show a warning and continue
|
||||
display.vvv("skipping vars_file '%s' due to an undefined variable" % vars_file_item)
|
||||
continue
|
||||
except AnsibleParserError:
|
||||
raise
|
||||
else:
|
||||
# if include_delegate_to is set to False, we ignore the missing
|
||||
# vars file here because we're working on a delegated host
|
||||
if include_delegate_to:
|
||||
raise AnsibleFileNotFound("vars file %s was not found" % vars_file_item)
|
||||
except (UndefinedError, AnsibleUndefinedVariable):
|
||||
if host is not None and self._fact_cache.get(host.name, dict()).get('module_setup') and task is not None:
|
||||
raise AnsibleUndefinedVariable("an undefined variable was found when attempting to template the vars_files item '%s'" % vars_file_item,
|
||||
obj=vars_file_item)
|
||||
else:
|
||||
# we do not have a full context here, and the missing variable could be because of that
|
||||
# so just show a warning and continue
|
||||
display.vvv("skipping vars_file '%s' due to an undefined variable" % vars_file_item)
|
||||
continue
|
||||
|
||||
display.vvv("Read vars_file '%s'" % vars_file_item)
|
||||
display.vvv("Read vars_file '%s'" % vars_file_item)
|
||||
except TypeError:
|
||||
raise AnsibleParserError("Error while reading vars files - please supply a list of file names. "
|
||||
"Got '%s' of type %s" % (vars_files, type(vars_files)))
|
||||
|
||||
# By default, we now merge in all vars from all roles in the play,
|
||||
# unless the user has disabled this via a config option
|
||||
|
|
Loading…
Reference in a new issue