Fix KeyError in s3fs _prune_deleted_files function

[GRomR1]

Fixes issue #68335 where fileserver.update fails with KeyError: 'Key' when using S3 buckets in multiple environments per bucket mode.

Problem

The issue occurs in the _prune_deleted_files function where the code assumes a different data structure than what is actually provided. When using multiple environments per bucket mode, the metadata structure is nested differently than expected.

Solution

The fix properly handles the nested bucket structure by iterating through buckets and objects within each bucket to extract the 'Key' field correctly.

Changes Made

1. Core Fix

• Modified _prune_deleted_files function in salt/fileserver/s3fs.py
• Fixed KeyError when processing metadata in multiple environments per bucket mode
• Added proper iteration through bucket structure to handle nested metadata

2. Code Improvements

• Applied reviewer feedback from @bdrx312 to make code more Pythonic
• Changed from meta.keys() to meta.values() for cleaner iteration
• Improved code readability while maintaining functionality

3. Test Coverage

• Added comprehensive tests for both s3fs configuration modes:
  • test_prune_deleted_files_multiple_envs_per_bucket - Tests the fix for multiple environments per bucket mode
  • test_prune_deleted_files_single_env_per_bucket - Tests single environment per bucket mode to prevent regression
• Tests verify:
  • Files are properly cached initially
  • Deleted files are removed from cache when calling _prune_deleted_files
  • Remaining files continue to exist in cache
  • The nested metadata structure is handled correctly in multiple env mode

Testing

This fix resolves the KeyError that was preventing fileserver.update from working with S3 buckets in multiple environments per bucket mode. The added tests ensure the fix works correctly and prevent future regressions.

Related Issues

Fixes #68335

Signed-off-by: GRomR1 [email protected]

[welcome]

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

• Salt's Contributor Guide
• Join our Community Discord
• Salt Project YouTube channel
• GitHub Discussions

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We're glad you've joined our community and look forward to doing awesome things with you!

[bdrx312]

To iterate over items (keyword, value pairs) in a dict, use .items(), but this isn't using the key so just use .values()

for objects in meta.values():
    for obj in objects:

alternatively use itertools:

import itertools

for obj in itertools.chain.from_iterable(meta.values()):

[GRomR1]

Thank you for the excellent feedback! You're absolutely right - using .values() is much more Pythonic and readable. I've applied your suggestion and updated the code accordingly.

The improved version is cleaner and more efficient since we're not using the dictionary keys anyway. I've created a new commit with this improvement: 38d24bc

Thanks for helping improve the code quality! 🙏

[twangboy]

Please write a test for this as well

[GRomR1]

twangboy Please write a test for this as well

Thank you for the feedback! I've added comprehensive tests for the s3fs _prune_deleted_files function fix.

The tests cover both configuration modes:

1. test_prune_deleted_files_multiple_envs_per_bucket - Tests the fix for multiple environments per bucket mode (the original issue [Bug]: Salt fileserver update does not work with S3 buckets #68335)
2. test_prune_deleted_files_single_env_per_bucket - Tests single environment per bucket mode to ensure no regression

Both tests verify that:

• Files are properly cached initially
• Deleted files are removed from cache when calling _prune_deleted_files
• Remaining files continue to exist in cache
• The nested metadata structure is handled correctly in multiple env mode

The tests are located in tests/pytests/unit/fileserver/test_s3fs.py and use the existing test infrastructure with moto for S3 mocking.

Please review the tests and let me know if any additional test coverage is needed.