Fix TypeError in file.recurse/directory clean when a require ID contains "file"#69760
Open
ggiesen wants to merge 1 commit into
Open
Fix TypeError in file.recurse/directory clean when a require ID contains "file"#69760ggiesen wants to merge 1 commit into
ggiesen wants to merge 1 commit into
Conversation
_gen_keep_files filtered requisites with `"file" in comp`, which for a bare-string requisite ID degraded to a substring match. Any ID containing the substring "file" then hit `comp["file"]` and raised "TypeError: string indices must be integers". Guard the membership test with an isinstance check so only dict requisites are considered; bare strings are ignored instead of crashing. Fixes saltstack#53692 and Fixes saltstack#61042
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Guards the requisite filter in
_gen_keep_files()(salt/states/file.py) with anisinstance(comp, dict)check so it only inspects dict-form requisites.The filter was
required_files = [comp for comp in require if "file" in comp]. For a bare-string requisite ID (written as- some_idrather than- file: some_id),"file" in compdegrades from a dict-key check into a substring test, so any ID containing the substring "file" (e.g.aaa_file,p_files_recurse_test_recurse_one) slips through. The loop then evaluatescomp["file"]on a str and raisesTypeError: string indices must be integers. The crash only fires when__lowstate__is non-empty.What issues does this PR fix or reference?
Fixes #53692
Fixes #61042
Previous Behavior
file.recurse/file.directoryrun withclean: Truecrashed withTypeError: string indices must be integerswhenever arequirerequisite was a bare state ID string whose ID contained the substring "file".New Behavior
Bare-string requisites are ignored by the keep-files filter instead of crashing. Normal dict requisites (
- file: <id>), which are the supported way to retain a required state's files during a clean, are unaffected and still contribute their files to the keep list.Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
changelog/53692.fixed.mdandchangelog/61042.fixed.mdtests/pytests/unit/states/file/test_find_keep_files.py: direct-altitude regression tests for both issues (bare-string ID containing "file" must return[], not raise), inverse must-not-regress tests proving dict requisites still retain their files, and peripheral coverage confirming a bare ID without "file" stays silently dropped.Commits signed with GPG?
No