Skip to content

fix(saltclass): deepcopy merged list and value overrides in dict_merge - #70031

Open
waterWang wants to merge 5 commits into
saltstack:masterfrom
waterWang:fix/saltclass-pillar-merge-duplicates
Open

fix(saltclass): deepcopy merged list and value overrides in dict_merge#70031
waterWang wants to merge 5 commits into
saltstack:masterfrom
waterWang:fix/saltclass-pillar-merge-duplicates

Conversation

@waterWang

@waterWang waterWang commented Aug 13, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes duplicate pillar entries in SaltClass pillar merging (issue #70022).

Root cause

When dict_merge is called with an empty accumulator dict a = {} and a source dict b = {"data": [...]}:

a[key] = b[key]  # reference assignment!

The a[key] becomes a shared reference to the original b[key] list. When a subsequent dict_merge call encounters the same key and both values are lists, it calls a[key].extend(b[key]) — which mutates the original b[key] through the shared reference.

In the SaltClass flow:

  1. Class test1's pillars are merged into __pillar__ (empty). __pillar__["data"] becomes a shared reference to test1's pillar list.
  2. Class test2's pillars are merged into __pillar__. __pillar__["data"].extend(test2_data) extends the shared list, polluting test1's original pillar data.
  3. The expanded class list now contains test1's pillars with test2's data appended. When both are merged again into the final pillars dict, test2's data appears twice.

Fix

Replace a[key] = b[key] with a[key] = copy.deepcopy(b[key]) in dict_merge to prevent shared reference mutation side effects. This is a single-line change that fixes the root cause.

What issues does this PR fix or reference?

Fixes #70022

In expanded_dict_from_minion(), the code was merging the entire exp_dict
(which includes 'pillars', 'states', 'classes' keys) into pillars_dict.
This caused the pillars dict to be nested under a 'pillars' key and
polluted with non-pillar keys.

Fix: merge only exp_dict["pillars"] into pillars_dict, and read
pillars_dict directly in get_pillars() instead of navigating through
the extra 'pillars' wrapper key.

Closes saltstack#70022
@waterWang
waterWang requested a review from a team as a code owner August 13, 2026 11:22
@welcome

welcome Bot commented Aug 13, 2026

Copy link
Copy Markdown

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at saltproject.pdl@broadcom.com. We're glad you've joined our community and look forward to doing awesome things with you!

Use copy.deepcopy when assigning values from dict b to dict a in
dict_merge, to prevent shared reference mutation side effects.

When dict_merge is called with an empty accumulator dict (a) and a
source dict (b), the assignment a[key] = b[key] creates a shared
reference. A subsequent call to dict_merge that extends a[key] (when
both are lists) will mutate the original b[key] as well, corrupting
the source data.

This was causing SaltClass pillar merging to produce duplicate
entries: the first class's pillar list was silently extended with
the second class's data, then both the polluted first class and the
correct second class contributed to the merged result.

Fixes saltstack#70022
@twangboy twangboy changed the title fix: merge only pillars dict, not the entire class dict in saltclass fix(saltclass): deepcopy merged list and value overrides in dict_merge Aug 13, 2026

@twangboy twangboy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please create a test and a changelog. Also, please rebase this against the 3006.x branch. We create bug fixes against the earliest supported branch where the bug exists.

@twangboy twangboy added the test:full Run the full test suite label Aug 13, 2026
@whytewolf

Copy link
Copy Markdown
Collaborator

the AI is strong with this one

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.

[Bug]: SaltClass duplicate entries in merged list

3 participants