Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[3006.x] salt-ssh pillar fixes #65484

Merged
merged 13 commits into from
Nov 26, 2023
Merged

[3006.x] salt-ssh pillar fixes #65484

merged 13 commits into from
Nov 26, 2023

Conversation

lkubb
Copy link
Contributor

@lkubb lkubb commented Oct 29, 2023 •
edited
Loading

What does this PR do?

  • Makes the salt-ssh state wrapper behave like the regular one, including not re-rendering an already rendered pillar when unnecessary and recursively merging overrides everywhere.
  • Ensures that when pillars are rerendered during lowstate compilation, master opts are present.
  • Ensures that pillars are rendered as in a regular Salt setup

What issues does this PR fix or reference?

Fixes: #62230
Fixes: #59802
Fixes: #65483
Fixes: #60002

Might fix:
#61861
#57121

Obviates the need for #57076

Previous Behavior

  • When states are applied, always renders pillar twice and performs 5-7 merges with itself by default (since the whole pillar is passed as an override).
  • Confusing behavior regarding pillar overrides (see linked issues).
  • Some pillar renderings might have worked for normal execution, but failed during state rendering because of the missing master opts.
  • Some state.* funcs would not have accepted pillar overrides, even if their usual counterpart would.
  • The master __opts__ during pillar rendering were not the same as during regular pillar rendering, causing several functions that rely on a fully functioning master with a populated cache to be borked, noticeably the gpg renderer, the git pillar and mine access, likely many more. (Details: Problem with salt-ssh and mine.get  #36796 (comment))

New Behavior

  • Reuses the pre-rendered pillar if no overrides/pillarenv is specified, only performs necessary merges with the passed overrides (plus ssh_merge_pillar behavior ofc).
  • Pillar data is in sync everywhere, except the minion (this is probably better introduced in a feature release, not as a bugfix).
  • Most pillar renderings should behave the same as during regular operation. The gpg renderer works, the regular mine can be accessed.

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated

Commits signed with GPG?

Yes

@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title salt-ssh pillar fixes [master] salt-ssh pillar fixes Oct 29, 2023
@lkubb lkubb temporarily deployed to ci October 29, 2023 19:41 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 19:42 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 19:42 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 19:42 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:00 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:00 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:49 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:50 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:50 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:50 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:50 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 29, 2023 20:50 — with GitHub Actions Inactive
@lkubb lkubb force-pushed the salt-ssh-state-pillar-override branch 4 times, most recently from e62b194 to 95facb5 Compare October 30, 2023 09:55
@lkubb lkubb changed the base branch from master to 3006.x October 30, 2023 09:56
@salt-project-bot-prod-environment salt-project-bot-prod-environment bot changed the title [master] salt-ssh pillar fixes [3006.x] salt-ssh pillar fixes Oct 30, 2023
@lkubb lkubb temporarily deployed to ci October 30, 2023 11:11 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 30, 2023 11:12 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 30, 2023 11:12 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 30, 2023 11:12 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 30, 2023 11:13 — with GitHub Actions Inactive
@lkubb lkubb temporarily deployed to ci October 30, 2023 11:13 — with GitHub Actions Inactive
@lkubb lkubb mentioned this pull request Oct 30, 2023
Closed
@lkubb
Update salt-ssh state wrapper pillar handling
ae2ae33 
Instead of passing the pre-rendered pillar as an override, do it like
the regular `state` execution module does with `salt-call`: Check if the
pillar needs to be rendered, otherwise reuse the already rendered one.

Also, ensure that __pillar__ in wrapper modules contains the same one
used during rendering, same thing for the one passed to `state.pkg`.

Also, ensure that when pillars are rerendered during a state run, they
get the master opts in addition to the minion ones, since some modules
used in the pillar can rely on them to be present.

Also, ensure pillar overrides are accepted for the same functions as with
the regular `state` execution module.
@lkubb
Fix salt-ssh master access during pillar rendering
38ba57d 
This also ports saltstack#50489 into the present
@lkubb lkubb force-pushed the salt-ssh-state-pillar-override branch from f0e6a0c to 38ba57d Compare October 30, 2023 19:27
@lkubb lkubb temporarily deployed to ci October 30, 2023 19:48 — with GitHub Actions Inactive
@lkubb lkubb requested a review from s0undt3ch November 26, 2023 16:24
@s0undt3ch s0undt3ch merged commit ab35c07 into saltstack:3006.x Nov 26, 2023
481 checks passed
This was referenced Dec 6, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s0undt3ch s0undt3ch s0undt3ch approved these changes

@twangboy twangboy twangboy left review comments

@garethgreenaway garethgreenaway Awaiting requested review from garethgreenaway garethgreenaway was automatically assigned from saltstack/team-core

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@lkubb @s0undt3ch @twangboy