Skip to content

Fix minion crash when grains config option is empty - #69891

Open
BrianHa94 wants to merge 2 commits into
saltstack:3006.xfrom
BrianHa94:fix-61321-minion-crash-on-invalid-empty-grains
Open

Fix minion crash when grains config option is empty#69891
BrianHa94 wants to merge 2 commits into
saltstack:3006.xfrom
BrianHa94:fix-61321-minion-crash-on-invalid-empty-grains

Conversation

@BrianHa94

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes a minion crash on startup caused by an empty grains: config option. YAML parses grains: with no value as None instead of a dict, which two separate code paths (apply_minion_config and salt.loader.grains(), which independently re-reads the raw config file off disk) assumed was always a dict. That crashed the minion with TypeError: 'NoneType' object is not iterable when building the __grains__ context wrapper.

Both locations now default the option to an empty dict ({}) when it resolves to None, and log a warning pointing out that grains: {} should be used instead of an empty grains: key.

What issues does this PR fix or reference?

Fixes #61321

Previous Behavior

A minion configured with an empty grains: config option (e.g. grains: with no value) would crash on startup with:

TypeError: 'NoneType' object is not iterable

raised from salt.utils.context.NamespacedDictWrapper.__init__ via the loader's __prep_mod_opts.

New Behavior

The minion starts normally. The empty grains option is treated as {}, and a warning is logged:

Config option 'grains' is set to an empty value. An empty 'grains' config is invalid, a dict is required. To set an empty grains config, use 'grains: {}' instead. Defaulting to an empty dict.

Merge requirements satisfied?

Add unit tests in tests/unit/test_config.py and tests/pytests/unit/loader/test_loader.py, plus an integration test in tests/pytests/integration/cli/test_salt_minion.py.

Commits signed with GPG?

No

@BrianHa94
BrianHa94 requested a review from a team as a code owner July 28, 2026 05:34
@BrianHa94 BrianHa94 added the test:full Run the full test suite label Jul 28, 2026
An empty 'grains:' config option parses to None instead of a dict,
which crashed the minion during startup with "TypeError: 'NoneType'
object is not iterable" when the loader tried to build the
__grains__ NamespacedDictWrapper.

Default the option to an empty dict in both places that read it:
apply_minion_config, and salt.loader.grains(), which independently
re-reads the raw config file off disk. Log a warning in each case
pointing out that 'grains: {}' should be used instead.

Fixes saltstack#61321
Comment thread salt/config/__init__.py Outdated
Comment thread tests/pytests/integration/cli/test_salt_minion.py Outdated
@twangboy twangboy added this to the Sulphur v3006.28 milestone Jul 28, 2026
Only warn/default the 'grains' config option when it is explicitly
present and None, not merely absent from opts. The looser
opts.get("grains") is None check could not distinguish "grains: set
to empty" from "grains key never set at all" (e.g. a sparse defaults
dict passed into apply_minion_config without a 'grains' key), causing
a false-positive warning in that case. This matches the equivalent
check already used in salt.loader.grains().

Also move log_file = factory.config["log_file"] in the new
integration test out of the try block, since it doesn't depend on the
test.ping call succeeding and reads more clearly next to the log file
assertions it's used for.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants