From 1ad6b08a0491e290ee30b4d488a1b1ffb13de3f3 Mon Sep 17 00:00:00 2001
From: Victor Zhestkov <Victor.Zhestkov@suse.com>
Date: Fri, 28 Oct 2022 13:19:23 +0300
Subject: [PATCH] Detect module.run syntax

* Detect module run syntax version

* Update module.run docs and add changelog

* Add test for module.run without any args

Co-authored-by: Daniel A. Wozniak <dwozniak@saltstack.com>
---
 changelog/58763.fixed                         |  1 +
 salt/states/module.py                         | 39 +++++++--
 .../pytests/functional/states/test_module.py  | 87 +++++++++++++++++++
 tests/unit/states/test_module.py              | 16 ++--
 4 files changed, 131 insertions(+), 12 deletions(-)
 create mode 100644 changelog/58763.fixed
 create mode 100644 tests/pytests/functional/states/test_module.py

diff --git a/changelog/58763.fixed b/changelog/58763.fixed
new file mode 100644
index 0000000000..53ee8304c0
--- /dev/null
+++ b/changelog/58763.fixed
@@ -0,0 +1 @@
+Detect new and legacy styles of calling module.run and support them both.
diff --git a/salt/states/module.py b/salt/states/module.py
index 380fda4fdc..a9801ea82b 100644
--- a/salt/states/module.py
+++ b/salt/states/module.py
@@ -4,16 +4,17 @@ Execution of Salt modules from within states
 
 .. note::
 
-    There are two styles of calling ``module.run``. **The legacy style will no
-    longer be available starting in the 3005 release.** To opt-in early to the
-    new style you must add the following to your ``/etc/salt/minion`` config
-    file:
+    As of the 3005 release, you no longer need to opt-in to the new style of
+    calling ``module.run``. The following config can be removed from ``/etc/salt/minion``:
 
     .. code-block:: yaml
 
         use_superseded:
           - module.run
 
+    Both 'new' and 'legacy' styles of calling ``module.run`` are supported.
+
+
 With `module.run` these states allow individual execution module calls to be
 made via states. Here's a contrived example, to show you how it's done:
 
@@ -299,13 +300,15 @@ Windows system:
 
 .. _file_roots: https://docs.saltproject.io/en/latest/ref/configuration/master.html#file-roots
 """
+import logging
 
 import salt.loader
 import salt.utils.args
 import salt.utils.functools
 import salt.utils.jid
 from salt.exceptions import SaltInvocationError
-from salt.utils.decorators import with_deprecated
+
+log = logging.getLogger(__name__)
 
 
 def wait(name, **kwargs):
@@ -337,7 +340,6 @@ def wait(name, **kwargs):
 watch = salt.utils.functools.alias_function(wait, "watch")
 
 
-@with_deprecated(globals(), "Phosphorus", policy=with_deprecated.OPT_IN)
 def run(**kwargs):
     """
     Run a single module function or a range of module functions in a batch.
@@ -370,6 +372,29 @@ def run(**kwargs):
 
     :return:
     """
+    # Detect if this call is using legacy or new style syntax.
+    legacy_run = False
+
+    keys = list(kwargs)
+    if "name" in keys:
+        keys.remove("name")
+
+    # The rest of the keys should be function names for new-style syntax
+    for name in keys:
+        if name.find(".") == -1:
+            legacy_run = True
+    if not keys and kwargs:
+        legacy_run = True
+
+    if legacy_run:
+        log.debug("Detected legacy module.run syntax: %s", __low__["__id__"])
+        return _legacy_run(**kwargs)
+    else:
+        log.debug("Using new style module.run syntax: %s", __low__["__id__"])
+        return _run(**kwargs)
+
+
+def _run(**kwargs):
 
     if "name" in kwargs:
         kwargs.pop("name")
@@ -482,7 +507,7 @@ def _call_function(name, returner=None, func_args=None, func_kwargs=None):
     return mret
 
 
-def _run(name, **kwargs):
+def _legacy_run(name, **kwargs):
     """
     .. deprecated:: 2017.7.0
        Function name stays the same, behaviour will change.
diff --git a/tests/pytests/functional/states/test_module.py b/tests/pytests/functional/states/test_module.py
new file mode 100644
index 0000000000..b93950c8b5
--- /dev/null
+++ b/tests/pytests/functional/states/test_module.py
@@ -0,0 +1,87 @@
+import logging
+
+import pytest
+from tests.support.helpers import dedent
+
+log = logging.getLogger(__name__)
+
+
+@pytest.mark.slow_test
+def test_issue_58763(tmp_path, modules, state_tree, caplog):
+
+    venv_dir = tmp_path / "issue-2028-pip-installed"
+
+    sls_contents = dedent(
+        """
+    run_old:
+      module.run:
+        - name: test.random_hash
+        - size: 10
+        - hash_type: md5
+
+    run_new:
+      module.run:
+        - test.random_hash:
+          - size: 10
+          - hash_type: md5
+    """
+    )
+    with pytest.helpers.temp_file("issue-58763.sls", sls_contents, state_tree):
+        with caplog.at_level(logging.DEBUG):
+            ret = modules.state.sls(
+                mods="issue-58763",
+            )
+            assert len(ret.raw) == 2
+            for k in ret.raw:
+                assert ret.raw[k]["result"] is True
+            assert "Detected legacy module.run syntax: run_old" in caplog.messages
+            assert "Using new style module.run syntax: run_new" in caplog.messages
+
+
+@pytest.mark.slow_test
+def test_issue_58763_a(tmp_path, modules, state_tree, caplog):
+
+    venv_dir = tmp_path / "issue-2028-pip-installed"
+
+    sls_contents = dedent(
+        """
+    test.random_hash:
+      module.run:
+        - size: 10
+        - hash_type: md5
+    """
+    )
+    with pytest.helpers.temp_file("issue-58763.sls", sls_contents, state_tree):
+        with caplog.at_level(logging.DEBUG):
+            ret = modules.state.sls(
+                mods="issue-58763",
+            )
+            assert len(ret.raw) == 1
+            for k in ret.raw:
+                assert ret.raw[k]["result"] is True
+            assert (
+                "Detected legacy module.run syntax: test.random_hash" in caplog.messages
+            )
+
+
+@pytest.mark.slow_test
+def test_issue_58763_b(tmp_path, modules, state_tree, caplog):
+
+    venv_dir = tmp_path / "issue-2028-pip-installed"
+
+    sls_contents = dedent(
+        """
+    test.ping:
+      module.run
+    """
+    )
+    with pytest.helpers.temp_file("issue-58763.sls", sls_contents, state_tree):
+        with caplog.at_level(logging.DEBUG):
+            ret = modules.state.sls(
+                mods="issue-58763",
+            )
+            assert len(ret.raw) == 1
+            print(ret)
+            for k in ret.raw:
+                assert ret.raw[k]["result"] is True
+            assert "Detected legacy module.run syntax: test.ping" in caplog.messages
diff --git a/tests/unit/states/test_module.py b/tests/unit/states/test_module.py
index 3ce20dfc32..a705bd3028 100644
--- a/tests/unit/states/test_module.py
+++ b/tests/unit/states/test_module.py
@@ -107,7 +107,13 @@ class ModuleStateTest(TestCase, LoaderModuleMockMixin):
     """
 
     def setup_loader_modules(self):
-        return {module: {"__opts__": {"test": False}, "__salt__": {CMD: MagicMock()}}}
+        return {
+            module: {
+                "__opts__": {"test": False},
+                "__salt__": {CMD: MagicMock()},
+                "__low__": {"__id__": "test"},
+            },
+        }
 
     @classmethod
     def setUpClass(cls):
@@ -170,7 +176,7 @@ class ModuleStateTest(TestCase, LoaderModuleMockMixin):
         with patch(
             "salt.utils.args.get_function_argspec", MagicMock(return_value=self.bspec)
         ):
-            ret = module._run(CMD, m_names="anyname")
+            ret = module._legacy_run(CMD, m_names="anyname")
         self.assertEqual(ret["comment"], "'names' must be a list.")
 
     def test_run_testmode(self):
@@ -382,7 +388,7 @@ class ModuleStateTest(TestCase, LoaderModuleMockMixin):
         name isn't available
         """
         with patch.dict(module.__salt__, {}, clear=True):
-            ret = module._run(CMD)
+            ret = module._legacy_run(CMD)
         self.assertFalse(ret["result"])
         self.assertEqual(
             ret["comment"], "Module function {} is not available".format(CMD)
@@ -393,7 +399,7 @@ class ModuleStateTest(TestCase, LoaderModuleMockMixin):
         Tests the return of module.run state when test=True is passed in
         """
         with patch.dict(module.__opts__, {"test": True}):
-            ret = module._run(CMD)
+            ret = module._legacy_run(CMD)
         self.assertEqual(
             ret["comment"], "Module function {} is set to execute".format(CMD)
         )
@@ -405,7 +411,7 @@ class ModuleStateTest(TestCase, LoaderModuleMockMixin):
         with patch(
             "salt.utils.args.get_function_argspec", MagicMock(return_value=self.aspec)
         ):
-            ret = module._run(CMD)
+            ret = module._legacy_run(CMD)
         self.assertIn("The following arguments are missing:", ret["comment"])
         self.assertIn("world", ret["comment"])
         self.assertIn("hello", ret["comment"])
-- 
2.38.0


