Fix stale resource cache, specific-name targeting, and add sync_resources

Three bugs fixed and one new feature added:
1. salt '*' test.ping was incorrectly dispatching to resources when
   targeting a specific minion by name (e.g. salt 'minion' test.ping).
   _resolve_resource_targets now only dispatches to resources for wildcard
   glob patterns (* ? [).  The master-side _augment_with_resources guard
   is updated consistently.
2. Removing a resource type from the pillar and running sync_all left
   stale resource IDs in the master cache indefinitely, causing phantom
   entries in salt '*' output.  Three co-operating fixes:
   - _discover_resources now distinguishes "no resources key in pillar"
     (preserve opts for config-file deployments) from "resources key
     present but empty" (authoritative removal — clear opts).
   - _register_resources_with_master no longer short-circuits on an empty
     resource dict; it always sends the registration so the master cache
     is overwritten with the current (possibly empty) state.
   - pillar_refresh re-discovers resources and re-registers with the
     master after successfully compiling a new pillar, so a sync_all
     picks up removals without requiring a minion restart.
3. _augment_with_resources had no error handling; a cache driver failure
   would propagate into check_minions and silently discard all PKI-based
   minion IDs.  Cache errors are now caught, logged, and degraded
   gracefully.
New: saltutil.sync_resources / refresh_resources — explicit resource
sync modelled after sync_modules/refresh_modules.  sync_all now includes
ret["resources"] and fires a resource_refresh event that triggers
_discover_resources + _register_resources_with_master on the minion.

Author: dwoz

salt/minion.py

@@ -561,19 +561,25 @@ def _discover_resources(self):
         ``discover(opts)``; the return value is a dict of
         ``{resource_type: [resource_id, ...]}``.
 
-        If pillar does not specify any resources, any resources already
-        present in ``opts["resources"]`` (e.g. set directly in the minion
-        config file) are preserved as-is.  This supports testing and simple
-        deployments where pillar-driven discovery is not needed.
+        If the pillar contains no ``resources`` key at all, any resources
+        already present in ``opts["resources"]`` (e.g. set directly in the
+        minion config file) are preserved as-is.  This supports testing and
+        simple deployments where pillar-driven discovery is not needed.
+
+        If the pillar *does* contain a ``resources`` key (even if its value is
+        empty / all entries removed), that is treated as an authoritative
+        declaration and the result will reflect only what the pillar says.
+        This ensures that removing a resource type from the pillar and running
+        sync_all actually clears it at runtime.
 
         Called from :meth:`gen_modules` after pillar is compiled and before
         the per-type execution-module loaders are created.
         """
-        pillar_resources = self.opts.get("pillar", {}).get("resources", {})
-        if not pillar_resources:
-            # No pillar resources — preserve whatever is already in opts
-            # (set directly in the minion config or from a previous discovery).
+        pillar = self.opts.get("pillar", {})
+        if "resources" not in pillar:
+            # Pillar has no opinion — preserve whatever is already in opts.
             return {k: list(v) for k, v in self.opts.get("resources", {}).items()}
+        pillar_resources = pillar.get("resources") or {}
 
         # A minimal resource loader is sufficient here — discover() only reads
         # from the opts dict passed to it and does not need other dunders.
@@ -3361,10 +3367,12 @@ async def _register_resources_with_master(self):
         ``minion_resources`` cache bank is up-to-date.  This allows
         :class:`salt.utils.minions.CkMinions` to include resource IDs when
         expanding glob / non-compound targets (e.g. ``salt '*' test.ping``).
+
+        An empty resource dict is sent deliberately when the minion has no
+        resources — this clears any stale entries left by a previous
+        registration (e.g. after a resource type is removed from the pillar).
         """
         resources = self.opts.get("resources", {})
-        if not resources:
-            return
         load = {
             "cmd": "_register_resources",
             "id": self.opts["id"],
@@ -3480,6 +3488,12 @@ async def pillar_refresh(self, force_refresh=False, clean_cache=False):
                 )
                 self.opts["pillar"] = new_pillar
                 self.functions.pack["__pillar__"] = self.opts["pillar"]
+                # Re-discover resources now that pillar has changed.  Must
+                # happen *after* opts["pillar"] is updated so that
+                # _discover_resources sees the new resource declarations (or
+                # their absence when a type is removed from the pillar).
+                self.opts["resources"] = self._discover_resources()
+                await self._register_resources_with_master()
             finally:
                 async_pillar.destroy()
         self.matchers_refresh()
@@ -3699,6 +3713,9 @@ async def handle_event(self, package):
             _minion.beacons_refresh()
         elif tag.startswith("matchers_refresh"):
             _minion.matchers_refresh()
+        elif tag.startswith("resource_refresh"):
+            _minion.opts["resources"] = _minion._discover_resources()
+            _minion.io_loop.create_task(_minion._register_resources_with_master())
         elif tag.startswith("manage_schedule"):
             _minion.manage_schedule(tag, data)
         elif tag.startswith("manage_beacons"):
@@ -4461,10 +4478,12 @@ def _resolve_resource_targets(self, load):
         Return the list of per-resource dicts ``{"id": ..., "type": ...}`` that
         the target expression matches against ``opts["resources"]``.
 
-        For glob/grain/etc. targets (non-resource-aware), returns all managed
-        resources so that ``salt '*' test.ping`` also runs against resources.
+        For wildcard glob targets (e.g. ``salt '*'``), returns all managed
+        resources so that the command also runs against resources.
         For compound T@ targets, returns only the matched resources.
-        For compound expressions with no T@ terms, returns an empty list.
+        For specific-name glob targets (e.g. ``salt 'minion'``), grain, list,
+        pillar, or compound expressions with no T@ terms, returns an empty list
+        — the operator is targeting the minion itself, not its resources.
         Internal/plumbing functions (see ``_NO_RESOURCE_FUNS``) are never
         dispatched to resources.
         """
@@ -4500,8 +4519,19 @@ def _resolve_resource_targets(self, load):
                             targets.append({"id": rid, "type": pattern})
             return targets
 
-        # Non-compound targets (glob, grain, list, etc.) apply to the minion
-        # AND to all managed resources.
+        # For glob targets, only dispatch to resources when the pattern
+        # contains a wildcard.  A bare name like ``salt 'minion' test.ping``
+        # targets the minion itself; it should not implicitly run against its
+        # resources.  ``salt '*' test.ping`` or ``salt 'web*' test.ping``
+        # opts in to resource dispatch.
+        if (
+            tgt_type == "glob"
+            and isinstance(tgt, str)
+            and not any(c in tgt for c in ("*", "?", "["))
+        ):
+            return []
+
+        # Wildcard glob — dispatch to all managed resources.
         all_resources = []
         for rtype, rids in resources.items():
             for rid in rids:

salt/modules/saltutil.py

@@ -402,52 +402,65 @@ def refresh_grains(**kwargs):
 
 def refresh_resources():
     """
-    Re-register this minion's managed resources with the master.
+    Signal the minion to re-discover its managed resources from current pillar
+    data and re-register them with the master.
 
-    Sends the current ``opts["resources"]`` to the master so that the master's
-    ``minion_resources`` cache bank is up-to-date.  This is the resource
-    analogue of ``saltutil.refresh_grains`` — call it whenever the set of
-    resources a minion manages may have changed, or after a minion restart to
-    ensure the master's targeting cache reflects the current state.
+    This fires a ``resource_refresh`` event on the minion bus.  The minion
+    handles the event by calling ``_discover_resources()`` (using the current
+    ``opts["pillar"]``) and then re-registering the result with the master's
+    ``minion_resources`` cache.
 
     CLI Example:
 
     .. code-block:: bash
 
         salt '*' saltutil.refresh_resources
     """
-    resources = __opts__.get("resources", {})
-    if not resources:
-        return True
+    try:
+        return __salt__["event.fire"]({}, "resource_refresh")
+    except KeyError:
+        return False
 
-    masters = []
-    if "master_uri_list" in __opts__:
-        masters.extend(__opts__["master_uri_list"])
-    else:
-        masters.append(__opts__["master_uri"])
 
-    from salt.exceptions import SaltReqTimeoutError
+def sync_resources(
+    saltenv=None, refresh=True, extmod_whitelist=None, extmod_blacklist=None
+):
+    """
+    Sync custom resource-type modules from ``salt://_resources`` to the minion
+    and signal the minion to re-discover its managed resources from pillar data
+    and re-register them with the master.
 
-    for master in masters:
-        with salt.channel.client.ReqChannel.factory(
-            __opts__, master_uri=master
-        ) as channel:
-            load = {
-                "cmd": "_register_resources",
-                "id": __opts__["id"],
-                "resources": resources,
-                "tok": channel.auth.gen_token(b"salt"),
-            }
-            try:
-                channel.send(load, timeout=60)
-                log.debug(
-                    "refresh_resources: registered %s with master %s",
-                    list(resources),
-                    master,
-                )
-            except SaltReqTimeoutError:
-                log.warning("refresh_resources: timed out contacting master %s", master)
-    return True
+    saltenv
+        The fileserver environment from which to sync. To sync from more than
+        one environment, pass a comma-separated list.
+
+        If not passed, then all environments configured in the :ref:`top files
+        <states-top>` will be checked for resource modules to sync. If no top
+        files are found, then the ``base`` environment will be synced.
+
+    refresh : True
+        If ``True``, signal the minion to re-discover its managed resources
+        and re-register them with the master. This refresh will be performed
+        even if no new resource modules are synced. Set to ``False`` to
+        prevent this refresh.
+
+    extmod_whitelist : None
+        comma-separated list of modules to sync
+
+    extmod_blacklist : None
+        comma-separated list of modules to blacklist
+
+    CLI Example:
+
+    .. code-block:: bash
+
+        salt '*' saltutil.sync_resources
+        salt '*' saltutil.sync_resources saltenv=base,dev
+    """
+    ret = _sync("resources", saltenv, extmod_whitelist, extmod_blacklist)
+    if refresh:
+        refresh_resources()
+    return ret
 
 
 def sync_grains(
@@ -1258,6 +1271,9 @@ def sync_all(
         saltenv, False, extmod_whitelist, extmod_blacklist
     )
     ret["matchers"] = sync_matchers(saltenv, False, extmod_whitelist, extmod_blacklist)
+    ret["resources"] = sync_resources(
+        saltenv, False, extmod_whitelist, extmod_blacklist
+    )
     if __opts__["file_client"] == "local":
         ret["pillar"] = sync_pillar(saltenv, False, extmod_whitelist, extmod_blacklist)
         ret["wrapper"] = sync_wrapper(

salt/utils/minions.py

@@ -798,18 +798,41 @@ def _augment_with_resources(self, minion_ids):
         IDs associated with the minions in ``minion_ids``.
 
         This is called by :meth:`check_minions` for non-compound targets so
-        that ``salt '*' test.ping`` or ``salt 'minion' test.ping`` also
-        includes returns from the minion's managed resources.
+        that ``salt '*' test.ping`` also includes returns from the minion's
+        managed resources.
+
+        If the resource cache is unavailable for any reason the method logs
+        an error and returns the original ``minion_ids`` unchanged so that
+        ordinary minion targeting is never disrupted by a resource-cache
+        failure.
         """
-        managed = self.cache.list("minion_resources") or []
+        try:
+            managed = self.cache.list("minion_resources") or []
+        except Exception:  # pylint: disable=broad-except
+            log.error(
+                "Failed to list minion_resources cache bank; "
+                "resource IDs will not be included in this target expansion. "
+                "Check cache driver configuration and permissions.",
+                exc_info=True,
+            )
+            return list(minion_ids)
         if not managed:
             return list(minion_ids)
         result = list(minion_ids)
         minion_set = set(minion_ids)
         for minion_id in managed:
             if minion_id not in minion_set:
                 continue
-            resources = self.cache.fetch("minion_resources", minion_id) or {}
+            try:
+                resources = self.cache.fetch("minion_resources", minion_id) or {}
+            except Exception:  # pylint: disable=broad-except
+                log.error(
+                    "Failed to fetch minion_resources for minion '%s'; "
+                    "skipping resource augmentation for this minion.",
+                    minion_id,
+                    exc_info=True,
+                )
+                continue
             for rids in resources.values():
                 for rid in rids:
                     if rid not in result:
@@ -861,9 +884,8 @@ def check_minions(
             # Compound targets handle resource matching explicitly via T@/M@.
             if (
                 tgt_type == "glob"
-                and tgt_type not in ("compound", "compound_pillar_exact")
                 and isinstance(expr, str)
-                and "*" in expr
+                and any(c in expr for c in ("*", "?", "["))
             ):
                 _res["minions"] = self._augment_with_resources(_res["minions"])
             _res["ssh_minions"] = False

tests/pytests/unit/test_minion_resources.py

@@ -56,12 +56,13 @@ def test_resolve_resource_targets_glob_wildcard(minion_with_resources):
 
 def test_resolve_resource_targets_glob_specific_minion(minion_with_resources):
     """
-    A specific minion name glob still returns all managed resources — the
-    minion decides what to dispatch based on the load, not the target string.
+    A specific name glob (no wildcard characters) must NOT dispatch to
+    resources.  ``salt 'minion' test.ping`` should only run on the minion
+    itself, not on its managed resources.
     """
     load = {"tgt": "minion", "tgt_type": "glob", "fun": "test.ping", "arg": []}
     targets = minion_with_resources._resolve_resource_targets(load)
-    assert len(targets) == 5
+    assert targets == [], "specific-name glob must not dispatch to resources"
 
 
 def test_resolve_resource_targets_compound_T_full_srn(minion_with_resources):
@@ -232,3 +233,67 @@ def run_job(name, target):
     assert not errors, errors
     assert results["a"] is target_a
     assert results["b"] is target_b
+
+
+# ---------------------------------------------------------------------------
+# _discover_resources tests
+# ---------------------------------------------------------------------------
+
+
+def test_discover_resources_no_pillar_key_preserves_opts(minion_with_resources):
+    """
+    When the pillar contains no 'resources' key at all, _discover_resources
+    preserves whatever is already in opts["resources"].
+    """
+    minion_with_resources.opts["pillar"] = {}
+    result = minion_with_resources._discover_resources()
+    assert result == {k: list(v) for k, v in RESOURCES.items()}
+
+
+def test_discover_resources_empty_pillar_key_clears_opts(minion_with_resources):
+    """
+    When the pillar *does* contain a 'resources' key but its value is empty,
+    _discover_resources must return {} and NOT preserve the old opts resources.
+    This is the fix for the stale-cache bug: removing a resource type from the
+    pillar and running sync_all must clear it at runtime.
+    """
+    minion_with_resources.opts["pillar"] = {"resources": {}}
+    result = minion_with_resources._discover_resources()
+    assert (
+        result == {}
+    ), "_discover_resources must return {} when pillar['resources'] is empty"
+
+
+# ---------------------------------------------------------------------------
+# _register_resources_with_master tests
+# ---------------------------------------------------------------------------
+
+
+def test_register_resources_with_master_sends_empty_dict(minion_with_resources):
+    """
+    _register_resources_with_master must send the registration even when
+    opts["resources"] is {}.  Without this, removing resources from the pillar
+    and running sync_all leaves the master cache permanently stale.
+    """
+    minion_with_resources.opts["resources"] = {}
+    sent_loads = []
+
+    async def fake_send(load, timeout=None):
+        sent_loads.append(load)
+
+    import asyncio
+
+    minion_with_resources.tok = b"test-tok"
+    with mock_patch.object(
+        minion_with_resources,
+        "_send_req_async_main",
+        side_effect=fake_send,
+    ):
+        asyncio.run(minion_with_resources._register_resources_with_master())
+
+    assert (
+        len(sent_loads) == 1
+    ), "_register_resources_with_master must always send a load, even for {}"
+    assert (
+        sent_loads[0]["resources"] == {}
+    ), "An empty resource dict must be forwarded to the master to clear stale cache"