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

オープン
#70,242 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 3 日以内に返信

まだ誰も着手していません。

評価

難易度
4/5
見積もり時間
3〜5日
初心者へのやさしさ
58/100
issue の種類
バグ
明瞭さ
おおむね明確
活発さ
活発
技術スタック
python

調査の方向性

salt/utils/optsdict.py の OptsDict.mutate_key() と DictProxy から始め、次に salt/loader/init.py の opts.get("grains", {}) のパッキングと、tests/pytests/unit/utils/test_optsdict.py の関連する単体テストを調査します。期待される動作を決定する前に、issue にある古いキーとキャッシュされた参照のケースを再現してください。削除された grains または pillar のキーが残存せず、文書化されたキャッシュ済み参照が更新を確認でき、回帰テストのカバレッジがある状態を完了とします。

索引モデルが issue の本文から書いたものです。

説明

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.

主要言語
Python
スター
15.8k
フォーク
5.6k
平均マージ
5日 9時間
マージ済み PR(30日)
99

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

saltstack/salt のほかの issue

saltstack/salt の issue をすべて見る

似ている issue

Python の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。