Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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.

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

[FEATURE REQUEST] Support dynamic expansion of file_roots and pillar_roots #64433

Closed
bluesliverx opened this issue Jun 6, 2023 · 0 comments · May be fixed by #64434
Closed

[FEATURE REQUEST] Support dynamic expansion of file_roots and pillar_roots #64433

bluesliverx opened this issue Jun 6, 2023 · 0 comments · May be fixed by #64434
Labels
Feature new functionality including changes to functionality and code refactors, etc.

Comments

@bluesliverx
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Similar to #6739, we use SPMs (with some improvements that I'll be submitting at some point soon here) extensively to install new formulas, pillars, etc to the Salt master. Due to the way that Salt handles file_roots and pillar_roots, the expansion of any globs for these paths are done once at startup. This works fine as long as we don't install any additional SPMs/formulas, but as soon as a new one is installed, we would have to restart the Salt master to re-expand the configured glob(s) in the roots.

We would like to make this dynamic so that every time file_roots/pillar_roots is accessed, it would be expanded with the globs so that new formulas/pillars/etc are detected and used.

Describe the solution you'd like
We have been utilizing a home-grown patch for Salt for about 5 years now (since Salt 2018) to make this work, and we're finally at the point where we want to contribute it back to the community. The solution uses a new DynamicDict class for file_roots, pillar_roots, and thorium_roots which acts like a normal dict to everything using it, but will actually call a method to expand the configured values on access of the values. It requires changes mainly to salt/config/__init__.py handling. I will be submitting the PR shortly for this.

Describe alternatives you've considered
We considered using gitfs, but this presented many problems in our environments (i.e. our git repos are only available from development machines and not in production). I also investigated if this functionality was present with config options such as roots_update_interval, but that seems to only affect the fileserver functionality and not the configured file roots on the master. I also considered using a new method like _validate_file_roots in salt/config/__init__.py that would be exposed to the rest of the salt infrastructure that would expand file roots in each case, but there may be many salt execution/state/etc modules that use opts["file_roots"] or such and those would be effectively broken in this case.

Additional context
Considering that we've been running this patch for 5 years in environments with up to roughly 30k+ minions and ~20 masters/syndics, it seems to not have detrimental effects on Salt itself. The glob expansion may slow down resolving file/pillar/thorium roots slightly, but my guess is it's not enough to outweigh the benefit of having dynamic config for it as this has been requested throughout the years in several issues and worked around with gitfs, etc.

This functionality is especially important for the SPM subsystem to function at all, since it was clearly intended to allow adding formulas on the fly (i.e. without restarting the Salt master), but maybe that is an assumption on our part.

Please Note
If this feature request would be considered a substantial change or addition, this should go through a SEP process here https://github.com/saltstack/salt-enhancement-proposals, instead of a feature request.

After looking through the SEP process, I don't believe the change is substantial enough to require this. If required, I can hide the functionality behind a feature flag, but since the only downside is the extra time required to expand globs when accessing roots, it felt right to make it the default functionality.

@bluesliverx bluesliverx added Feature new functionality including changes to functionality and code refactors, etc. needs-triage labels Jun 6, 2023
@anilsil anilsil added this to the Chlorine v3007.0 milestone Jun 7, 2023
@dwoz dwoz unassigned Ch3LL Jul 29, 2024
@saltstack saltstack locked and limited conversation to collaborators Feb 5, 2025
@dwoz dwoz converted this issue into discussion #67290 Feb 5, 2025

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Feature new functionality including changes to functionality and code refactors, etc.
Projects
None yet
4 participants