saltstack / saltstack/salt

[Bug]: OptsDict.mutate_key()'s clear() is a no-op (DictProxy has no clear override) -- removed grains/pillar keys never actually clear

Open
#70,242 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

What happened?

salt.utils.optsdict.OptsDict.mutate_key() is documented as replacing a dict-valued key's contents in place ("clear + update", i.e. a full replace) specifically so that already-cached references to opts["grains"] / opts["pillar"] see the update without needing opts[key] to be rebound to a new object. This is the only way salt/minion.py ever applies a real grains or pillar refresh (opts.mutate_key("grains", new_grains) / opts.mutate_key("pillar", new_pillar) -- there is no opts["grains"] = new_grains path left in the minion refresh flow).

The "clear" half of mutate_key() is silently a no-op, so it actually behaves like a plain merge/update, never a replace:

# salt/utils/optsdict.py, OptsDict.mutate_key()
if key in self and isinstance(self[key], dict) and isinstance(new_value, dict):
    # Mutate in place to preserve object identity
    self[key].clear()
    self[key].update(new_value)
else:
    self[key] = new_value

self[key] goes through OptsDict.__getitem__, which wraps a dict-valued key in a DictProxy -- a point-in-time snapshot/copy-on-write wrapper around the real underlying value, not the underlying dict itself. DictProxy does not override clear() (it does override update(), pop(), setdefault(), __setitem__, __delitem__). So self[key].clear() calls the inherited plain dict.clear(), which empties only that particular DictProxy instance's own storage -- the persistent target (OptsDict._local[key] / the shared base value) is never touched, and the proxy is discarded immediately afterward with no other reference to it. The very next line, self[key].update(new_value), re-evaluates self[key], which reads the (still fully populated, never-cleared) persistent target and merges new_value into it.

Net effect: any key present in the old grains/pillar value but absent from the freshly computed new_value is never actually removed -- it survives every subsequent mutate_key() call indefinitely. On a live minion this means a grain (or pillar key) that legitimately disappears between refreshes -- a custom grain module removed, hardware unplugged, a pillar top-file entry deleted -- keeps reporting its last known value forever, with no cache-bust and no error, because nothing about the refresh path or its return value indicates a partial/failed replace.

A second, related effect from the same root cause: salt/loader/__init__.py's grain_funcs()/minion_mods() pack the __grains__ dunder injected into every loaded grains/execution module as opts.get("grains", {}) (L621 and L1282 on master). OptsDict.get() just calls self[key], so this also returns a disposable DictProxy snapshot, not the live-updating reference mutate_key()'s own docstring claims callers get ("Loaders cache references to opts['grains'], opts['pillar'], etc. ... Cached references continue to see updates"). A __grains__ dunder captured at module-load time is frozen at that snapshot and does not reflect a later mutate_key("grains", ...) refresh.

I confirmed both effects reproduce identically against the actual current master branch copy of salt/utils/optsdict.py (fetched via the GitHub API, run standalone -- it has no non-stdlib dependencies), not just the installed 3008.2 release. I also checked tests/pytests/unit/utils/test_optsdict.py on master; it exercises mutate_key-adjacent deepcopy/identity behavior but has no test asserting mutate_key()'s clear+update/full-replace semantics, so this doesn't appear to be intentionally-tested current behavior.

Minimal repro (pure Python, no minion/master needed)
from salt.utils.optsdict import OptsDict

opts = OptsDict.from_dict({
    "grains": {"osrelease": "7", "stale_removed_grain": "should-be-gone"}
})
print("before:", dict(opts["grains"]))

# Exactly how salt/minion.py applies every real grains refresh:
#     new_grains = salt.loader.grains(self.opts, ...)
#     self.opts.mutate_key("grains", new_grains)
# The grain source has legitimately stopped producing 'stale_removed_grain'.
new_grains = {"osrelease": "8"}
opts.mutate_key("grains", new_grains)

after = dict(opts["grains"])
print("after:", after)
print("removed grain incorrectly still present:", "stale_removed_grain" in after)
# -> True: {'osrelease': '8', 'stale_removed_grain': 'should-be-gone'}

Ran twice (fresh interpreter each time) against both the installed salt==3008.2 package and the fetched master-branch optsdict.py standalone -- identical, deterministic output every time: the removed key survives.

The second effect (stale __grains__ dunder):

from salt.utils.optsdict import OptsDict

opts = OptsDict.from_dict({"grains": {"osrelease": "7"}})
# Mirrors salt/loader/__init__.py: pack = {"__grains__": opts.get("grains", {}), ...}
packed_grains_for_module = opts.get("grains", {})

opts.mutate_key("grains", {"osrelease": "8"})

print(dict(packed_grains_for_module))  # {'osrelease': '7'} -- stale
print(dict(opts["grains"]))            # {'osrelease': '8'} -- correct, fresh read
Expected behavior

mutate_key(key, new_value) should fully replace opts[key]'s contents with new_value (matching its documented "clear + update" behavior) -- a key present in the old value but absent from new_value should be gone afterward. Any reference obtained via the documented "cached references continue to see updates" contract (i.e. via ordinary opts[key] / opts.get(key) reads, such as the __grains__ dunder packed for execution modules) should actually observe subsequent mutate_key() updates, or the docstring's claim should be corrected to describe the real (snapshot) semantics.

Type of salt install

pip (pypi)

Major version

3008.x (also confirmed unchanged against master)

What supported OS are you seeing the problem on?

Not OS-specific -- this is pure-Python dict/proxy logic in salt/utils/optsdict.py, reproduced on Ubuntu (WSL2). Confirmed via gh api that OptsDict.mutate_key() and the DictProxy class (no clear()/no live-reference override) are unchanged on saltstack/salt's master branch as of 2026-09-04.

salt --versions-report output
Salt Version:
          Salt: 3008.2

Python Version:
        Python: 3.14.4 (main, Apr  8 2026, 04:02:31) [GCC 15.2.0]

Dependency Versions:
          cffi: 2.1.1
      cherrypy: 18.10.0
  cryptography: 50.0.1
      dateutil: 2.9.0.post0
         gitdb: 4.0.12
     gitpython: 3.1.61
        Jinja2: 3.1.6
  looseversion: 1.3.0
       msgpack: 1.2.2
     packaging: 24.0
     pycparser: 3.00
  pycryptodome: 3.23.0
  python-gnupg: 0.5.6
        PyYAML: 6.0.3
         PyZMQ: 27.2.0

Found via a property-based audit (copy/clone independence, cache-invalidation consistency) run across several widely-used Python libraries; repro was written and independently re-run before filing.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with OptsDict.mutate_key() and DictProxy in salt/utils/optsdict.py, then inspect the opts.get("grains", {}) packing in salt/loader/init.py and the related unit tests in tests/pytests/unit/utils/test_optsdict.py. Reproduce the stale-key and cached-reference cases from the issue before determining the expected behavior. Done means removed grains or pillar keys no longer persist and documented cached references observe refreshes, with regression coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
devops, infrastructure
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.