Skip to content

Include the offending path in file module directory errors#69691

Open
ggiesen wants to merge 2 commits into
saltstack:3006.xfrom
ggiesen:fix-47707-file-invalid-dir-msg
Open

Include the offending path in file module directory errors#69691
ggiesen wants to merge 2 commits into
saltstack:3006.xfrom
ggiesen:fix-47707-file-invalid-dir-msg

Conversation

@ggiesen

@ggiesen ggiesen commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

file.readdir and file.rmdir raised a path-less "A valid directory was not specified." SaltInvocationError, making template/highstate failures hard to diagnose (issue #47707). The fix embeds the offending path in both messages (f-string with the local path variable at salt/modules/file.py:4195 and :4345). Added two tests asserting the path appears in the message; verified both FAIL on unpatched code (via git stash of the source fix) and PASS with the fix. Full test file: 6 passed. Note: the shared venv's salt-factories log-server/event-listener plugins hang collection, so they were disabled with -p no: flags; shadow-import assert confirmed the worktree salt was under test.

What issues does this PR fix or reference?

Fixes #47707

Previous Behavior

See #47707.

New Behavior

Include the offending path in file module directory errors. Validated by a unit test proven to fail on unpatched 3006.x and pass with the fix (confirmed by adversarial review).

Merge requirements satisfied?

  • Tests written/updated
  • Changelog

Commits signed with GPG?

No

The "A valid directory was not specified." SaltInvocationError raised by
file.readdir and file.rmdir omitted the offending path, making template
and highstate failures hard to diagnose. Include the path in the message.

Fixes saltstack#47707
…d-dir errors

The direct test calls file.rmdir with the exact argument shape the
file.rmdir state uses (recurse=..., verbose=True, older_than=...), since
verbose=True normally routes removal failures into the returned dict
instead of raising, and proves the invalid-directory error still raises
with the offending path in the message. The inverse tests guard against
overcorrection: relative paths must still hit the untouched absolute-path
check for both rmdir and readdir, and readdir on an existing directory
must still return its listing without raising.
@dwoz dwoz added the test:full Run the full test suite label Jul 7, 2026
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.

2 participants