Fix state.apply ignoring saltenv when autoloading dynamic modules

[matthewdva]

What does this PR do?

Makes a saltenv-pinned state run sync dynamic modules (_modules, _states, _grains, ...) from that saltenv and nothing else.

BaseHighState.load_dynamic() passed every saltenv present in the top file match dict to saltutil.sync_all:

syncd = self.state.functions["saltutil.sync_all"](list(matches), refresh=False)

matches is not limited to the requested saltenv. Extra saltenvs get in two ways:

• _master_tops() results, which top_matches() merged with no saltenv filter — even though the top file sections ~25 lines earlier are filtered against opts["saltenv"];
• cross-saltenv - <saltenv>: <sls> entries in a top file.

That would be harmless if each saltenv had its own destination, but salt.utils.extmods.sync() copies every saltenv into one flat extension_modules/<form>/ directory. There is no per-saltenv separation, so the last saltenv in the list overwrites the earlier ones — silently, and it stays overwritten for subsequent runs.

This also contradicts what doc/topics/development/modules/index.rst already documents:

When dynamic modules are autoloaded via states, only the modules defined in the same saltenv as the states currently being run are synced.

What issues does this PR fix or reference?

Fixes #67069

Previous Behavior

From #67069 — state.apply saltenv=dev writes the dev copy of a custom state, then overwrites it with the base copy:

[INFO    ] Caching directory '_states/' for environment 'dev'
[INFO    ] Copying '/var/cache/salt/minion/files/dev/_states/sideboard.py' to '/var/cache/salt/minion/extmods/states/sideboard.py'
[INFO    ] Syncing states for environment 'base'
[INFO    ] Caching directory '_states/' for environment 'base'
[INFO    ] Copying '/var/cache/salt/minion/files/base/_states/sideboard.py' to '/var/cache/salt/minion/extmods/states/sideboard.py'

The net effect, with a custom module foo.py that differs between base and qa:

salt <minion> saltutil.sync_all saltenv=qa    # foo.py v1.1 cached  -- correct
salt <minion> state.apply saltenv=qa          # re-syncs, ends up with v1.0  -- wrong
salt <minion> state.apply saltenv=qa <state>  # no sync, uses cached v1.1  -- correct

The targeted form works only because state.sls does not autoload; it just uses whatever was cached.

New Behavior

state.apply saltenv=qa syncs dynamic modules from qa only.

• salt/state.py — load_dynamic(): when opts["saltenv"] is set, sync from exactly that saltenv. With no explicit saltenv the old behavior is kept and every matched saltenv is synced. state.highstate with no saltenv leaves opts["saltenv"] as None, so that path stays reachable and unchanged.

• salt/state.py — top_matches(): skip master_tops data for saltenvs other than the requested one, matching the filter already applied to top file sections. Please look at this hunk specifically — it is the one behavior change beyond module syncing. If an ENC / master_tops returns base states and an operator wants those in a saltenv=qa run, this drops them. It is self-contained and can be dropped if you would rather not change that; the load_dynamic() hunk alone fixes the reported bug.

• salt/utils/extmods.py — sync(): warn when a module name exists in more than one synced saltenv, naming the winner. Pure logging — the flat extension_modules directory and its last-one-wins precedence are deliberately left alone, since making it per-saltenv would be a much larger change touching the loader. This just makes the collision visible instead of silent.

Merge requirements satisfied?

• Docs — doc/topics/development/modules/index.rst documents the saltenv-pinned behavior and the shared extension_modules directory
• Changelog — changelog/67069.fixed.md
• Tests written/updated — tests/pytests/unit/state/test_load_dynamic.py (6) and tests/pytests/unit/utils/test_extmods.py (4)

The tests were verified to be genuine regression tests: with salt/state.py and salt/utils/extmods.py reverted, 4 of the 10 fail; with the fix, all 10 pass. The other 6 cover the unchanged no-explicit-saltenv path, guarding against over-correcting it.

Full tests/pytests/unit/ suite was run with and without the patch on 3008.x; the failure sets are identical, so nothing here regresses. (The residual failures are pre-existing on a clean checkout and environment-specific — local pygit2 version, macOS-only tests, zeromq timing.)

Notes for reviewers

• Branch choice: originally opened against 3006.x per CONTRIBUTING.rst ("the oldest supported branch where the bug exists"); retargeted to 3008.x at maintainer request, since 3006.x is now CVE-only and 3007.15 was the final 3007 release. The code sites are unchanged on master (salt/utils/extmods.py is byte-identical), and the reporter in [BUG] salt state.apply syncs base modules irrespective of saltenv during salt runs #67069 was running 3006.9.
• The branch is a single commit on top of 3008.x. The only difference from the 3006.x version is that it no longer touches the existing sentence in doc/topics/development/modules/index.rst: on 3006.x that sentence was incomplete and had to be fixed, while 3008.x already has the complete wording.

Commits signed with GPG?

No

[matthewdva]

@twangboy Thanks for the approval in August. This was moved from the 3006.28 milestone to 3008.5 in the post-release sweep without being merged, so it missed 3006.28, 3007.15 and 3008.3. I've merged the latest 3006.x into the branch (clean merge, no changes to the patch); the CI run is waiting on maintainer approval to start. Is there anything else you need from me to get it in?

The bug is also present on 3007.x, 3008.x and master. Given the milestone is now 3008.5, should I retarget this to 3008.x, or open separate PRs for 3007.x/3008.x so it doesn't depend on merge-forward? (I noticed #70342/#70343 rescuing 3006.x fixes that didn't make it forward.) I already have a tested 3007.x version of the branch.

@EliAndrewC has also confirmed the fix in production on #67069.

[twangboy]

Yes, we are no longer accepting bug fixes on 3006.x. Please retarget the 3008.x branch.

[matthewdva]

Only 3008? How about 3007, which is really where I'd like to see this land?

[twangboy]

We've released a capstone release of 3007.15. That is the final release. Bug fixes now go in 3008.x. 3006.x is CVE only.

The base branch was changed.

[matthewdva]

Thanks for clarifying. Retargeted to 3008.x.

The branch is now a single commit on top of 3008.x. The only change from the 3006.x version is dropping the doc wording fix that was specific to 3006.x, since 3008.x already has the complete sentence; the code, tests and changelog are unchanged. Re-verified on 3008.x: without the fix, 4 of the 10 new tests fail; with it, all 10 pass; and the full tests/pytests/unit/ suite has no new failures compared to a clean 3008.x checkout.

@twangboy Changing the base dismissed your earlier approval, so this needs a fresh review. CI will also need approval to run.