Add capability to specify state args as a dict

Also add some more type hints and change some format calls to f strings.
Fix state retry tests with invalid assumptions of duration and comment.

Author: bdrx312

changelog/68367.added.md

@@ -0,0 +1 @@
+Added capability to specify module arguments in an sls as a dict

salt/state.py

@@ -7,18 +7,13 @@
 
 import pytest
 
-import salt.loader
 import salt.modules.cmdmod as cmd
 import salt.modules.config as config
 import salt.modules.grains as grains
 import salt.modules.saltutil as saltutil
 import salt.modules.state as state_mod
-import salt.utils.atomicfile
-import salt.utils.files
-import salt.utils.path
 import salt.utils.platform
 import salt.utils.state as state_util
-import salt.utils.stringutils
 
 log = logging.getLogger(__name__)
 
@@ -646,6 +641,36 @@ def test_pydsl(state, state_tree, tmp_path):
         assert testfile.exists()
 
 
+def test_sls_with_state_args_dict(state, state_tree):
+    """
+    Call sls file with state argument as a dict.
+    """
+    sls_contents = """
+    A:
+      test.succeed_without_changes:
+        name: echo foo
+    """
+    with pytest.helpers.temp_file("testing.sls", sls_contents, state_tree):
+        ret = state.sls("testing")
+        for staterun in ret:
+            assert staterun.result is True
+
+
+def test_sls_with_state_args_list(state, state_tree):
+    """
+    Call sls file with state argument as a dict instead of a list.
+    """
+    sls_contents = """
+    A:
+      test.succeed_without_changes:
+        - name: echo foo
+    """
+    with pytest.helpers.temp_file("testing.sls", sls_contents, state_tree):
+        ret = state.sls("testing")
+        for staterun in ret:
+            assert staterun.result is True
+
+
 def test_issues_7905_and_8174_sls_syntax_error(state, state_tree):
     """
     Call sls file with yaml syntax error.
@@ -680,12 +705,15 @@ def test_issues_7905_and_8174_sls_syntax_error(state, state_tree):
         "badlist1.sls", badlist_1_sls_contents, state_tree
     ), pytest.helpers.temp_file("badlist2.sls", badlist_2_sls_contents, state_tree):
         ret = state.sls("badlist1")
-        assert ret.failed
-        assert ret.errors == ["State 'A' in SLS 'badlist1' is not formed as a list"]
+        staterun = ret["cmd_|-A_|-A_|-run"]
+        assert staterun.result is False
+        assert staterun.changes["stderr"] == "/bin/sh: line 1: A: command not found"
 
         ret = state.sls("badlist2")
         assert ret.failed
-        assert ret.errors == ["State 'C' in SLS 'badlist2' is not formed as a list"]
+        assert ret.errors == [
+            "State 'C' in SLS 'badlist2' is not formed as a list or dict"
+        ]
 
 
 def test_retry_option(state, state_tree):
@@ -737,7 +765,7 @@ def test_retry_option_is_true(state, state_tree):
         for state_return in ret:
             assert state_return.result is False
             assert expected_comment in state_return.comment
-            assert state_return.full_return["duration"] >= 3
+            assert state_return.full_return["duration"] >= 30
 
 
 @pytest.mark.skip_initial_gh_actions_failure(skip=_check_skip)
@@ -769,7 +797,7 @@ def test_retry_option_success(state, state_tree, tmp_path):
             assert state_return.result is True
             assert state_return.full_return["duration"] < duration
             # It should not take 2 attempts
-            assert "Attempt 2" not in state_return.comment
+            assert "Attempt 1" not in state_return.comment
 
 
 @pytest.mark.skip_on_windows(
@@ -820,41 +848,44 @@ def test_retry_option_eventual_success(state, state_tree, tmp_path):
     testfile1 = tmp_path / "testfile-1"
     testfile2 = tmp_path / "testfile-2"
 
+    interval = 2
+
     def create_testfile(testfile1, testfile2):
         while True:
             if testfile1.exists():
                 break
-        time.sleep(2)
+        time.sleep(interval)
         testfile2.touch()
 
     thread = threading.Thread(target=create_testfile, args=(testfile1, testfile2))
-    sls_contents = """
+    sls_contents = f"""
     file_test_a:
       file.managed:
-        - name: {}
+        - name: {testfile1}
         - content: 'a'
 
     file_test:
       file.exists:
-        - name: {}
+        - name: {testfile2}
         - retry:
             until: True
             attempts: 5
-            interval: 2
+            interval: {interval}
             splay: 0
         - require:
           - file_test_a
-    """.format(
-        testfile1, testfile2
-    )
+    """
     with pytest.helpers.temp_file("retry.sls", sls_contents, state_tree):
         thread.start()
         ret = state.sls("retry")
-        for state_return in ret:
+        for num, state_return in enumerate(ret):
             assert state_return.result is True
-            assert state_return.full_return["duration"] > 4
-            # It should not take 5 attempts
-            assert "Attempt 5" not in state_return.comment
+            assert state_return.full_return["duration"] > interval
+            if num == 1:
+                # It should retry at least 1 time
+                assert "Attempt 1" in state_return.comment
+                # It should not take 5 attempts
+                assert "Attempt 5" not in state_return.comment
 
 
 @pytest.mark.skip_on_windows(
@@ -867,45 +898,48 @@ def test_retry_option_eventual_success_parallel(state, state_tree, tmp_path):
     testfile1 = tmp_path / "testfile-1"
     testfile2 = tmp_path / "testfile-2"
 
+    interval = 2
+
     def create_testfile(testfile1, testfile2):
         while True:
             if testfile1.exists():
                 break
-        time.sleep(2)
+        time.sleep(interval)
         testfile2.touch()
 
     thread = threading.Thread(target=create_testfile, args=(testfile1, testfile2))
-    sls_contents = """
+    sls_contents = f"""
     file_test_a:
       file.managed:
-        - name: {}
+        - name: {testfile1}
         - content: 'a'
 
     file_test:
       file.exists:
-        - name: {}
+        - name: {testfile2}
         - retry:
             until: True
             attempts: 5
-            interval: 2
+            interval: {interval}
             splay: 0
         - parallel: True
         - require:
           - file_test_a
-    """.format(
-        testfile1, testfile2
-    )
+    """
     with pytest.helpers.temp_file("retry.sls", sls_contents, state_tree):
         thread.start()
         ret = state.sls(
             "retry", __pub_jid="1"
         )  # Because these run in parallel we need a fake JID
-        for state_return in ret:
+        for num, state_return in enumerate(ret):
             log.debug("=== state_return %s ===", state_return)
             assert state_return.result is True
-            assert state_return.full_return["duration"] > 4
-            # It should not take 5 attempts
-            assert "Attempt 5" not in state_return.comment
+            assert state_return.full_return["duration"] > interval
+            if num == 1:
+                # It should retry at least 1 time
+                assert "Attempt 1" in state_return.comment
+                # It should not take 5 attempts
+                assert "Attempt 5" not in state_return.comment
 
 
 def test_state_non_base_environment(state, state_tree_prod, tmp_path):
@@ -1131,8 +1165,8 @@ def test_it(name):
         """
         This should not fail on spawning platforms:
           requires_env.test_it:
-            - name: foo
-            - parallel: true
+            name: foo
+            parallel: true
         """
     )
     with pytest.helpers.temp_file(

tests/pytests/functional/modules/state/test_state.py

@@ -81,6 +81,28 @@ def test_local_sls_call(salt_master, salt_call_cli):
         assert state_run_dict["changes"]["ret"] == "hello"
 
 
+def test_local_sls_call_with_dict_args(salt_master, salt_call_cli):
+    sls_contents = """
+    regular-module:
+      module.run:
+        name: test.echo
+        text: hello
+    """
+    with salt_master.state_tree.base.temp_file("saltcalllocal.sls", sls_contents):
+        ret = salt_call_cli.run(
+            "--local",
+            "--file-root",
+            str(salt_master.state_tree.base.paths[0]),
+            "state.sls",
+            "saltcalllocal",
+        )
+        assert ret.returncode == 0
+        state_run_dict = next(iter(ret.data.values()))
+        assert state_run_dict["name"] == "test.echo"
+        assert state_run_dict["result"] is True
+        assert state_run_dict["changes"]["ret"] == "hello"
+
+
 def test_local_salt_call(salt_call_cli):
     """
     This tests to make sure that salt-call does not execute the

tests/pytests/integration/cli/test_salt_call.py

@@ -50,16 +50,16 @@ def test_avoid_injecting_shell_code_as_root(
     cmd = "echo $(id -u)"
 
     ret = salt_call_cli.run("cmd.run_stdout", cmd, python_shell=True)
-    root_id = ret.json
+    root_id = ret.data
     ret = salt_call_cli.run(
         "cmd.run_stdout", cmd, runas=running_username, python_shell=True
     )
-    runas_root_id = ret.json
+    runas_root_id = ret.data
 
     ret = salt_call_cli.run(
         "cmd.run_stdout", cmd, runas=non_root_account.username, python_shell=True
     )
-    user_id = ret.json
+    user_id = ret.data
 
     assert user_id != root_id
     assert user_id != runas_root_id

tests/pytests/integration/modules/test_cmdmod.py

@@ -15,14 +15,12 @@ def test_logging_and_state_output_order(salt_master, salt_minion, salt_cli, tmp_
     """
     target_path = tmp_path / "file-target.txt"
     sls_name = "file-target"
-    sls_contents = """
+    sls_contents = f"""
     add_contents_pillar_sls:
       file.managed:
-        - name: {}
+        - name: {target_path}
         - contents: foo
-    """.format(
-        target_path
-    )
+    """
     sls_tempfile = salt_master.state_tree.base.temp_file(
         f"{sls_name}.sls", sls_contents
     )

tests/pytests/integration/modules/test_state.py

@@ -1323,8 +1323,8 @@ def test_directory_recurse(salt_master, salt_call_cli, tmp_path, grains):
     with sls_tempfile:
         ret = salt_call_cli.run("state.sls", sls_name)
         key = f"file_|-{target_dir}_|-{target_dir}_|-directory"
-        assert key in ret.json
-        result = ret.json[key]
+        assert key in ret.data
+        result = ret.data[key]
         assert "changes" in result and result["changes"]
 
     # Permissions of file should not have changed.

tests/pytests/integration/states/test_file.py

@@ -46,7 +46,30 @@ def state_name(salt_master):
         yield name
 
 
-def test_sls(salt_call_cli, salt_master, state_name):
+@pytest.fixture
+def state_name_dict_arg(salt_master):
+    name = "some-test-state"
+    sls_contents = """
+    test_foo:
+      test.succeed_with_changes:
+          name: foo
+    """
+    with salt_master.state_tree.base.temp_file(f"{name}.sls", sls_contents):
+        if not platform.is_windows() and not platform.is_darwin():
+            subprocess.run(
+                [
+                    "chown",
+                    "-R",
+                    "salt:salt",
+                    str(salt_master.state_tree.base.write_path),
+                ],
+                check=False,
+            )
+        yield name
+
+
+@pytest.mark.parametrize("fixture_name", ["state_name", "state_name_dict_arg"])
+def test_sls(salt_call_cli, salt_master, fixture_name):
     """
     Test calling a sls file
     """

tests/pytests/pkg/integration/test_salt_call.py

@@ -205,7 +205,7 @@ def test_compiler_pad_funcs_short_sls(minion_opts, tmp_path):
                 }
             },
             [
-                "State 'master_pub' in SLS '/srv/reactor/start.sls' is not formed as a list"
+                "State 'master_pub' in SLS '/srv/reactor/start.sls' is not formed as a list or dict"
             ],
         ),
         (