Adjust other consumers of get_function_argspec

Author: lkubb

salt/fileserver/__init__.py

@@ -504,7 +504,7 @@ def envs(self, back=None, sources=False):
             fstr = f"{fsb}.envs"
             kwargs = (
                 {"ignore_cache": True}
-                if "ignore_cache" in _argspec(self.servers[fstr]).args
+                if "ignore_cache" in _argspec(self.servers[fstr]).namedargs
                 and self.opts["__role"] == "minion"
                 else {}
             )

salt/key.py

@@ -4,7 +4,6 @@
 """
 
 import fnmatch
-import itertools
 import logging
 import os
 import shutil
@@ -146,14 +145,8 @@ def _get_args_kwargs(self, fun, args=None):
         if args is None:
             args = []
             if argspec.args:
-                # Iterate in reverse order to ensure we get the correct default
-                # value for the positional argument.
-                for arg, default in itertools.zip_longest(
-                    reversed(argspec.args), reversed(argspec.defaults or ())
-                ):
+                for arg, default in argspec.argdefaults.items():
                     args.append(self.opts.get(arg, default))
-            # Reverse the args so that they are in the correct order
-            args = args[::-1]
 
         if argspec.keywords is None:
             kwargs = {}

salt/master.py

@@ -2494,7 +2494,7 @@ def _prep_pub(self, minions, jid, clear_load, extra, missing):
 
                 # Check if 'minions' is included in returner's save_load arg_spec.
                 # This may be missing in custom returners, which we should warn about.
-                if "minions" not in arg_spec.args:
+                if "minions" not in arg_spec.namedargs:
                     log.critical(
                         "The specified returner used for the external job cache "
                         "'%s' does not have a 'minions' kwarg in the returner's "
@@ -2522,7 +2522,7 @@ def _prep_pub(self, minions, jid, clear_load, extra, missing):
         # always write out to the master job caches
         try:
             fstr = "{}.save_load".format(self.opts["master_job_cache"])
-            self.mminion.returners[fstr](clear_load["jid"], clear_load, minions)
+            self.mminion.returners[fstr](clear_load["jid"], clear_load, minions=minions)
         except KeyError:
             log.critical(
                 "The specified returner used for the master job cache "

salt/modules/saltutil.py

@@ -2009,7 +2009,7 @@ def runner(
 
     if name in rclient.functions:
         aspec = salt.utils.args.get_function_argspec(rclient.functions[name])
-        if "saltenv" in aspec.args:
+        if "saltenv" in aspec.namedargs:
             kwarg["saltenv"] = saltenv
 
     if name in ["state.orchestrate", "state.orch", "state.sls"]:
@@ -2095,7 +2095,7 @@ def wheel(name, *args, **kwargs):
     try:
         if name in wheel_client.functions:
             aspec = salt.utils.args.get_function_argspec(wheel_client.functions[name])
-            if "saltenv" in aspec.args:
+            if "saltenv" in aspec.namedargs:
                 valid_kwargs["saltenv"] = saltenv
 
         if jid:

salt/pillar/__init__.py

@@ -1147,10 +1147,12 @@ def _external_pillar_data(self, pillar, val, key):
         Builds actual pillar data structure and updates the ``pillar`` variable
         """
         ext = None
-        args = salt.utils.args.get_function_argspec(self.ext_pillars[key]).args
+        valid_kwargs = salt.utils.args.get_function_argspec(
+            self.ext_pillars[key]
+        ).namedargs
 
         if isinstance(val, dict):
-            if ("extra_minion_data" in args) and self.extra_minion_data:
+            if ("extra_minion_data" in valid_kwargs) and self.extra_minion_data:
                 ext = self.ext_pillars[key](
                     self.minion_id,
                     pillar,
@@ -1160,7 +1162,7 @@ def _external_pillar_data(self, pillar, val, key):
             else:
                 ext = self.ext_pillars[key](self.minion_id, pillar, **val)
         elif isinstance(val, list):
-            if ("extra_minion_data" in args) and self.extra_minion_data:
+            if ("extra_minion_data" in valid_kwargs) and self.extra_minion_data:
                 ext = self.ext_pillars[key](
                     self.minion_id,
                     pillar,
@@ -1170,7 +1172,7 @@ def _external_pillar_data(self, pillar, val, key):
             else:
                 ext = self.ext_pillars[key](self.minion_id, pillar, *val)
         else:
-            if ("extra_minion_data" in args) and self.extra_minion_data:
+            if ("extra_minion_data" in valid_kwargs) and self.extra_minion_data:
                 ext = self.ext_pillars[key](
                     self.minion_id,
                     pillar,

salt/state.py

@@ -1498,19 +1498,9 @@ def verify_data(self, data):
         else:
             # First verify that the parameters are met
             aspec = salt.utils.args.get_function_argspec(self.states[full])
-            arglen = 0
-            deflen = 0
-            if isinstance(aspec.args, list):
-                arglen = len(aspec.args)
-            if isinstance(aspec.defaults, tuple):
-                deflen = len(aspec.defaults)
-            for ind in range(arglen - deflen):
-                if aspec.args[ind] not in data:
-                    errors.append(
-                        "Missing parameter {} for state {}".format(
-                            aspec.args[ind], full
-                        )
-                    )
+            for req in aspec.allreq:
+                if req not in data:
+                    errors.append(f"Missing parameter {req} for state {full}")
         # If this chunk has a recursive require, then it will cause a
         # recursive loop when executing, check for it
         reqdec = ""

salt/states/service.py

@@ -103,9 +103,9 @@ def _get_systemd_only(func, kwargs):
 
     ret = {}
     warnings = []
-    valid_args = _argspec(func).args
+    valid_kwargs = _argspec(func).namedargs
     for systemd_arg in SYSTEMD_ONLY:
-        if systemd_arg in kwargs and systemd_arg in valid_args:
+        if systemd_arg in kwargs and systemd_arg in valid_kwargs:
             if _get_systemd_only.HAS_SYSTEMD:
                 ret[systemd_arg] = kwargs[systemd_arg]
             else:

salt/utils/args.py

@@ -335,6 +335,24 @@ def kwonlyreq(self) -> tuple[str, ...]:
         kwdefaults = self.kwonlydefaults
         return tuple(kw for kw in self.kwonlyargs if kw not in kwdefaults)
 
+    @property
+    def allreq(self) -> tuple[str, ...]:
+        """
+        Tuple of parameters of any kind that require an argument.
+        """
+        return tuple(list(self.argreq) + list(self.kwonlyreq))
+
+    @property
+    def namedargs(self) -> tuple[str, ...]:
+        """
+        Tuple of parameters that can be passed by name.
+        The positional equivalent to this is just ``args``.
+        """
+        return tuple(
+            [arg for arg in self.args if arg not in self.posonlyargs]
+            + list(self.kwonlyargs)
+        )
+
 
 def get_function_argspec(func, is_class_method=None) -> _ArgSpec:
     """

tests/pytests/integration/states/test_arg_kinds.py

@@ -92,9 +92,12 @@ def test_kwonly_defaults_required_kwonly_missing(salt_call_cli, salt_master):
     with salt_master.state_tree.base.temp_file("kwonly_defaults_miss.sls", contents):
         ret = salt_call_cli.run("state.apply", "kwonly_defaults_miss")
     assert ret.returncode != 0, ret
-    assert any(
-        "missing 1 required keyword-only" in stream
-        for stream in (ret.stdout, ret.stderr)
+    # Ensure state verification catches it, not salt.utils.args.format_call
+    single_ret = ret.data[next(iter(ret.data))]
+    assert single_ret["result"] is False
+    assert (
+        "Missing parameter c for state arg_kinds.kwonly_defaults"
+        in single_ret["comment"]
     )
 
 

tests/pytests/unit/utils/test_args.py

@@ -302,8 +302,20 @@ def test_get_function_argspec(fun, expected):
         "kwdefaults": {},
         "posonlydefaults": {},
         "alldefaults": {},
+        "allreq": (),
+        "namedargs": (),
     }
     expected = defaults | expected
+    # Fill allreq/namedargs from expected data to avoid repetition
+    if expected["argreq"] or expected["kwonlyreq"]:
+        expected["allreq"] = expected["allreq"] or tuple(
+            list(expected["argreq"]) + list(expected["kwonlyreq"])
+        )
+    if expected["args"] or expected["kwonlyargs"]:
+        expected["namedargs"] = expected["namedargs"] or tuple(
+            list(arg for arg in expected["args"] if arg not in expected["posonlyargs"])
+            + list(expected["kwonlyargs"])
+        )
     spec = salt.utils.args.get_function_argspec(fun)
     for attr, exp in expected.items():
         assert getattr(spec, attr) == exp