Skip to content

Conversation

@wirwolf
Copy link

@wirwolf wirwolf commented Dec 1, 2025

Problem

The mod_data function in salt/client/ssh/__init__.py was only discovering modules from the fileserver backend (e.g., _modules, _states directories accessed via fsclient.cache_file()). This caused salt-ssh to ignore:

  • Custom modules registered via entry-points
  • Modules from extension_modules configuration
  • Modules from module_dirs CLI options
  • Custom fileserver backends and other loader plugins

This resulted in inconsistent behavior between salt-call (which loads all extensions) and salt-ssh (which ignores them).

Solution

Modified mod_data() to collect modules from three sources:

1. Global Loader Paths

Uses salt.loader._module_dirs() to discover all module directories, including:

  • Entry-point registered plugins
  • extension_modules from configuration
  • module_dirs from CLI
  • System-wide module paths

2. File Roots

Directly scans file_roots directories for _modules, _states, etc., avoiding network-dependent fileserver operations that require master connectivity.

3. Error Handling

Wraps all discovery operations in try-except blocks to ensure salt-ssh continues working even if:

  • Some module paths are inaccessible
  • Entry-point plugins fail to load
  • File roots are misconfigured

Benefits

  • Consistency: salt-ssh now uses the same module discovery logic as salt-call
  • Entry-point support: Custom plugins registered via setuptools entry-points are now available
  • No master dependency: Avoids KeyError: 'master_uri' by not using fileserver client operations
  • Resilience: Graceful error handling ensures partial failures don't break salt-ssh

Technical Details

Before: Only scanned fileserver via fsclient.cache_file() → required master connection

After:

  1. Calls salt.loader._module_dirs(opts, ref, tag=...) for each module type
  2. Scans directories directly from filesystem
  3. Collects modules from file_roots configuration
  4. Builds tarball with all discovered modules for deployment to targets

Key Changes:

  • Removed dependency on fsclient.cache_file() which triggers master authentication
  • Added direct filesystem scanning of file_roots
  • Added comprehensive exception handling
  • Preserved backward compatibility with existing module discovery

Impact

All salt-ssh deployments now have access to:

  • Custom execution modules
  • Custom states
  • Custom grains
  • Custom renderers
  • Custom returners
  • Custom fileserver backends (when registered via entry-points)

This resolves the inconsistency where salt-call --local sys.list_fileserver_backends would show custom backends but salt-ssh <target> sys.list_fileserver_backends would not.

Commits signed with GPG?

Yes

Linked issue: #68496

@wirwolf wirwolf requested a review from a team as a code owner December 1, 2025 15:50
@twangboy twangboy added the test:full Run the full test suite label Dec 4, 2025
@twangboy twangboy added this to the Sulfur v3006.18 milestone Dec 4, 2025
Copy link
Contributor

@twangboy twangboy left a comment

Choose a reason for hiding this comment

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

Please add a changelog and write a test

## Problem

The `mod_data` function in `salt/client/ssh/__init__.py` was only discovering modules from the fileserver backend (e.g., `_modules`, `_states` directories accessed via `fsclient.cache_file()`). This caused salt-ssh to ignore:

- Custom modules registered via entry-points
- Modules from `extension_modules` configuration
- Modules from `module_dirs` CLI options
- Custom fileserver backends and other loader plugins

This resulted in inconsistent behavior between `salt-call` (which loads all extensions) and `salt-ssh` (which ignores them).

## Solution

Modified `mod_data()` to collect modules from three sources:

### 1. Global Loader Paths
Uses `salt.loader._module_dirs()` to discover all module directories, including:
- Entry-point registered plugins
- `extension_modules` from configuration
- `module_dirs` from CLI
- System-wide module paths

### 2. File Roots
Directly scans `file_roots` directories for `_modules`, `_states`, etc., avoiding network-dependent fileserver operations that require master connectivity.

### 3. Error Handling
Wraps all discovery operations in try-except blocks to ensure salt-ssh continues working even if:
- Some module paths are inaccessible
- Entry-point plugins fail to load
- File roots are misconfigured

## Benefits

- **Consistency**: salt-ssh now uses the same module discovery logic as salt-call
- **Entry-point support**: Custom plugins registered via setuptools entry-points are now available
- **No master dependency**: Avoids `KeyError: 'master_uri'` by not using fileserver client operations
- **Resilience**: Graceful error handling ensures partial failures don't break salt-ssh

## Technical Details

**Before**: Only scanned fileserver via `fsclient.cache_file()` → required master connection

**After**: 
1. Calls `salt.loader._module_dirs(opts, ref, tag=...)` for each module type
2. Scans directories directly from filesystem
3. Collects modules from `file_roots` configuration
4. Builds tarball with all discovered modules for deployment to targets

**Key Changes**:
- Removed dependency on `fsclient.cache_file()` which triggers master authentication
- Added direct filesystem scanning of `file_roots`
- Added comprehensive exception handling
- Preserved backward compatibility with existing module discovery

## Impact

All salt-ssh deployments now have access to:
- Custom execution modules
- Custom states
- Custom grains
- Custom renderers
- Custom returners
- Custom fileserver backends (when registered via entry-points)

This resolves the inconsistency where `salt-call --local sys.list_fileserver_backends` would show custom backends but `salt-ssh <target> sys.list_fileserver_backends` would not.
@wirwolf wirwolf force-pushed the patch-1 branch 2 times, most recently from 87dd0c5 to 4d11ac9 Compare December 10, 2025 11:11
@wirwolf
Copy link
Author

wirwolf commented Dec 10, 2025

Please add a changelog and write a test

Done
PS. I am not sure of a clear understanding of where I need to add changelog

@twangboy
Copy link
Contributor

The documentation is here:
https://docs.saltproject.io/en/latest/topics/development/changelog.html

Basically, create a file in the changelog directory and the changelog will be updated on the next release based on the content of that file. There are some naming conventions depending on what this PR is doing. This one would probably be 68496.fixed.md.

@wirwolf
Copy link
Author

wirwolf commented Dec 10, 2025

The documentation is here: https://docs.saltproject.io/en/latest/topics/development/changelog.html

Basically, create a file in the changelog directory and the changelog will be updated on the next release based on the content of that file. There are some naming conventions depending on what this PR is doing. This one would probably be 68496.fixed.md.

Fixed

twangboy
twangboy previously approved these changes Dec 10, 2025
@twangboy
Copy link
Contributor

If you install pre-commit (pip install pre-commit) you can run it before you commit. In your case, I would add a blank line somewhere in the files where pre-commit is failing then run the following:

git add .
pre-commit

Pre-commit will run and make all the changes you're seeing in the failing pre-commit. Repeat those 2 commands until pre-commit comes back clean.

@wirwolf
Copy link
Author

wirwolf commented Dec 12, 2025

Hello @twangboy. Please re-run tests for Windows. I see he has failed because some services are sent
Invoke-WebRequest : 504 Gateway Time-out

@dwoz dwoz merged commit cd2e438 into saltstack:3006.x Dec 13, 2025
1161 of 1189 checks passed
@welcome
Copy link

welcome bot commented Dec 13, 2025

Congratulations on your first PR being merged! 🎉

@wirwolf wirwolf deleted the patch-1 branch December 14, 2025 09:13
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]: salt-ssh does not load custom loader modules while salt-call does

3 participants