[master] Fix 64433: Add dynamic loading of file_roots, pillar_roots, and thorium_roots

[bluesliverx]

What does this PR do?

Adds dynamic expansion of file/pillar/thorium roots config.

What issues does this PR fix or reference?

Fixes: #64433

Previous Behavior

The roots were expanded only once at startup.

New Behavior

The roots are expanded on every access of the environments within the file_roots, pillar_roots, and thorium_roots options.

Merge requirements satisfied?

• Docs
• Changelog - https://docs.saltproject.io/en/master/topics/development/changelog.html
• Tests written/updated

Commits signed with GPG?

No

[Ch3LL]

Can we get some additional test coverage that actually tests the full functionality. For example, adding a new directory path to file roots and being able to call that SLS file and same for pillar.

I'd like to also get some additional reviews on this.

[bluesliverx]

@Ch3LL I have added tests and updated the documentation as requested. Let me know if you see anything else.

[bluesliverx]

@twangboy, this is ready now I believe.

[bluesliverx]

How does one go about getting this merged? :)

[twangboy]

I'm thinking we'll get things fixed on 3006.x and 3007.x, then merge those forward into master. Then we can rebase and get this in.

[bluesliverx]

@twangboy sorry to be annoying, any update on getting this merged in? Just curious.

[bluesliverx]

@twangboy I am getting really annoying, I'm sure. But any help getting this merged in time for 3008 would be much appreciated so we can stop maintaining our own patch :)

[twangboy]

When we get closer to release we will start merging pending PRs into master branch that are passing tests.

[twangboy]

Please rebase this PR and fix conflicts

[bluesliverx]

@twangboy done

[dwoz]

@bluesliverx needs rebase

[bluesliverx]

@dwoz rebased

[twangboy]

Please address the merge conflicts

[bluesliverx]

@twangboy done

[twangboy]

Critical Performance Bug: Uncached Disk I/O on Dictionary Access
The Issue: Every time a core Salt subsystem reads an environment path (e.g., checking opts["file_roots"]["base"]), __getitem__ triggers the _expand_glob_path callback. This function performs expensive, blocking file system I/O using glob.glob(path).

The Impact: During a highstate run or compilation, the options dictionary is accessed hundreds or thousands of times. Running uncached disk globbing loops repeatedly on every single key lookup will severely degrade master performance at scale.

The Fix: Implement a short-lived cache (TTL mechanism) inside DynamicDict so that the expensive glob evaluation hits the physical disk at a reasonable interval rather than on every single key read.

A clean way to solve this is to add an internal cache dictionary inside DynamicDict that tracks a timestamp alongside the evaluated paths, limiting disk scans to once every few seconds.

Modify the DynamicDict class in salt/utils/dynamic_dict.py:

import time
import glob

class DynamicDict(dict):
    def __init__(self, *args, **argv):
        self._func_dict = {}
        self._cache = {}
        self._ttl = 5.0  # Cache disk globs for 5 seconds
        super().__init__(*args, **argv)

    def __getitem__(self, key):
        val = super().__getitem__(key)
        if key in self._func_dict:
            now = time.time()
            # If cache is expired or missing, re-evaluate
            if key not in self._cache or (now - self._cache[key]["ts"]) > self._ttl:
                evaluated_val = self._func_dict[key](val, dyn_dict=self, key=key)
                self._cache[key] = {"val": evaluated_val, "ts": now}
            
            return self._cache[key]["val"]
        return val

    def __delitem__(self, key):
        if key in self._func_dict:
            del self._func_dict[key]
        if key in self._cache:
            del self._cache[key]
        super().__delitem__(key)

A Better Architectural Approach
Instead of a hardcoded time-based TTL, a cleaner, more reliable approach for Salt config structures is to use a cached property that flushes when the loader reloads, or a slightly longer, configurable default window (like 5 to 10 seconds).

If you want to keep the time-based approach but make it resilient against mid-job expiration, you can expose the TTL as a master configuration option that defaults to 5.0 seconds:

self._ttl = opts.get("dynamic_dict_ttl", 5.0)

This gives you a safer buffer for long-running compilations while allowing teams with massive environments to turn it up if their disk I/O becomes a bottleneck.

[twangboy]

Critical API Mismatch: .items() Omission and Broken Dictionary Views

The Issue: The DynamicDict implementation completely fails to override .items()`, and it implements .values()`` as a custom generator rather than a standard Python 3 dictionary view object (dict_values).

The Impact: Because .items() falls back to super().items(), running loops like for env, paths in opts["file_roots"].items(): bypasses the dynamic wrapper entirely, returning the raw, unglobbed static strings instead of the expanded paths. Furthermore, calling list(sdb_opts.items()) in functions like apply_sdb fails to evaluate the dynamic keys, while code expecting standard dictionary view behaviors (such as set operations) will break when interacting with a raw generator.

The Fix: Explicitly override keys(), values(), and items() inside DynamicDict to evaluate the dynamic pathways cleanly and ensure parity with standard Python 3 dictionary behaviors.

Add the following methods to the DynamicDict class in salt/utils/dynamic_dict.py, replacing the existing values() generator:

    def keys(self):
        return super().keys()

    def values(self):
        # Return a custom list-like view or tuple list to mirror dict_values
        return [self[key] for key in super().keys()]

    def items(self):
        # Crucial fix: Ensures loops over .items() evaluate the dynamic paths
        return [(key, self[key]) for key in super().keys()]

[twangboy]

Redundant Assignment Bug in __deepcopy__
The Issue: Inside the else block of the __deepcopy__ method, the implementation executes rdd[key] = value. This assigns the uncopied references directly to the new dictionary, only for the very next line to completely overwrite it with a proper copy.deepcopy() operation.

The Impact: The initial assignment line is entirely redundant, adds unnecessary dict-write overhead during deep copy operations, and leaves confusing dead code in a core utility module.

The Fix: Clean up the else block by removing the redundant, non-copied assignment line entirely, ensuring the method handles copying cleanly.

Refactor the __deepcopy__ loop in salt/utils/dynamic_dict.py like this:

    def __deepcopy__(self, memo):
        rdd = DynamicDict()
        memo[id(self)] = rdd
        
        # Walk the underlying raw dictionary keys directly
        for key in super().keys():
            if key in self._func_dict:
                func = self._func_dict[key]
                data = super().__getitem__(key)
                rdd.add_dyn(key, func, data)
            else:
                # Fix: Strip out the redundant non-copied assignment line
                copied_key = copy.deepcopy(key, memo)
                copied_value = copy.deepcopy(super().__getitem__(key), memo)
                rdd[copied_key] = copied_value
                
        return rdd