Improve error message with multiple string-valued state args

This commit is contained in:
jeanluc 2023-09-19 00:14:19 +02:00 committed by Megan Wilhite
parent c14310b77e
commit 93859b1e6c
3 changed files with 82 additions and 19 deletions

1
changelog/38098.fixed.md Normal file
View file

@ -0,0 +1 @@
Improved error message when state arguments are accidentally passed as a string

View file

@ -486,17 +486,21 @@ class Compiler:
fun += 1
for arg in body[state]:
if isinstance(arg, str):
fun += 1
if " " in arg.strip():
if "." in state:
errors.append(
'The function "{}" in state '
'"{}" in SLS "{}" has '
"whitespace, a function with whitespace is "
"not supported, perhaps this is an argument "
'that is missing a ":"'.format(
arg, name, body["__sls__"]
)
"Argument not formed as a dictionary in state "
f"'{name}' in SLS '{body['__sls__']}': '{arg}'"
)
else:
fun += 1
if " " in arg.strip():
errors.append(
f'The function "{arg}" in state '
f'"{name}" in SLS "{body["__sls__"]}" has '
"whitespace, a function with whitespace is "
"not supported, perhaps this is an argument"
' that is missing a ":"'
)
elif isinstance(arg, dict):
# The arg is a dict, if the arg is require or
# watch, it must be a list.
@ -597,8 +601,11 @@ class Compiler:
)
elif fun > 1:
errors.append(
"Too many functions declared in state '{}' in "
"SLS '{}'".format(state, body["__sls__"])
f"Too many functions declared in state '{state}' in "
f"SLS '{body['__sls__']}'. Please choose one of the following: "
+ ", ".join(
arg for arg in body[state] if isinstance(arg, str)
)
)
return errors
@ -1509,14 +1516,22 @@ class State:
fun += 1
for arg in body[state]:
if isinstance(arg, str):
fun += 1
if " " in arg.strip():
if "." in state:
errors.append(
'The function "{}" in state "{}" in SLS "{}" has '
"whitespace, a function with whitespace is not "
"supported, perhaps this is an argument that is "
'missing a ":"'.format(arg, name, body["__sls__"])
"Argument not formed as a dictionary in state "
f"'{name}' in SLS '{body['__sls__']}': '{arg}'"
)
else:
fun += 1
if " " in arg.strip():
errors.append(
f'The function "{arg}" in state '
f'"{name}" in SLS "{body["__sls__"]}" has '
"whitespace, a function with whitespace is "
"not supported, perhaps this is an argument"
' that is missing a ":"'
)
elif isinstance(arg, dict):
# The arg is a dict, if the arg is require or
# watch, it must be a list.
@ -1615,8 +1630,11 @@ class State:
)
elif fun > 1:
errors.append(
"Too many functions declared in state '{}' in "
"SLS '{}'".format(state, body["__sls__"])
f"Too many functions declared in state '{state}' in "
f"SLS '{body['__sls__']}'. Please choose one of the following: "
+ ", ".join(
arg for arg in body[state] if isinstance(arg, str)
)
)
return errors

View file

@ -1099,3 +1099,47 @@ def test_verify_onlyif_cmd_opts_exclude(minion_opts):
timeout=5,
success_retcodes=1,
)
@pytest.mark.parametrize("verifier", (salt.state.State, salt.state.Compiler))
@pytest.mark.parametrize(
"high,err_msg",
(
(
{"/some/file": {"file.managed": ["source:salt://bla"]}},
"Argument not formed as a dictionary",
),
(
{"/some/file": {"file": ["managed", "source:salt://bla"]}},
"Please choose one of the following: managed, source:salt",
),
),
)
def test_verify_high_too_many_functions_declared_error_message(
high, err_msg, minion_opts, verifier
):
"""
Ensure the error message when a list item of a state call is
accidentally passed as a string instead of a single-item dict
is more meaningful. Example:
/some/file:
file.managed:
- source:salt://bla
/some/file:
file:
- managed
- source:salt://bla
Issue #38098.
"""
high[next(iter(high))]["__sls__"] = "sls"
with patch("salt.state.State._gather_pillar"):
if verifier is salt.state.Compiler:
state_obj = verifier(minion_opts, [])
else:
state_obj = verifier(minion_opts)
res = state_obj.verify_high(high)
assert isinstance(res, list)
assert any(err_msg in x for x in res)