Skip to content

Conversation

@vikash-s1
Copy link
Member

Issue #, if available:

N/A - New feature implementation

Description of changes:

[Draft PR] Adding SafeRemoveAll feature to validate filesystem mounts while deletion.

This PR introduces a new SafeRemoveAll wrapper function in the internal/system package that provides safe directory removal with mount point detection and optional unmounting capabilities. This replaces all os.RemoveAll() calls throughout the codebase to prevent accidental deletion of mounted filesystems.

Key Changes:

Core Implementation:

  • New SafeRemoveAll function with configurable behavior:
    • allowUnmount: Controls whether to attempt unmounting detected mount points
    • forceUnmount: Enables aggressive unmount methods when graceful unmount fails
  • Cross-platform support with Linux-specific and fallback implementations
  • Mount point detection for both directories and files using k8s.io/mount-utils
  • Optimized directory traversal with early termination for mount points
  • Retry logic with exponential backoff for unmount operations
  • Verification to ensure mount points are successfully unmounted

Platform-Specific Files:

  • safe_remove_linux.go: Linux syscall-based force unmount (MNT_DETACH, MNT_FORCE)
  • safe_remove_other.go: Fallback implementation for non-Linux platforms
  • Graceful fallback to os.RemoveAll() when mount utilities are unsupported

Codebase Integration:

  • Replaced 15+ os.RemoveAll() calls across multiple packages:
    • internal/cleanup, internal/flows, internal/kubectl
    • internal/kubelet, internal/cni, internal/containerd
    • internal/ssm, internal/tracker, internal/artifact
    • internal/iamrolesanywhere, internal/iamauthenticator
  • All calls currently use SafeRemoveAll(path, false, false) (safe mode)
  • Ready for future extension to enable unmount capabilities

Dependencies:

  • Added k8s.io/mount-utils v0.33.2 for cross-platform mount operations

Testing (if applicable):

Unit Tests:

  • Comprehensive test suite (safe_remove_test.go)
    • Normal removal without mount points
    • Mount point detection and refusal to delete
    • Successful unmount and removal workflows
    • Platform fallback behavior
    • Error handling and retry logic
    • Both directory and file mount point scenarios
    • Benchmark tests for performance validation

Integration Tests:

  • New integration test: uninstall-with-mounted-filesystem
  • Tests real uninstall scenarios with SafeRemoveAll integration
  • Helper functions added to helpers.sh for mount point testing
  • Cross-platform compatibility verified in Docker container environment
  • Ready for extension when unmount parameters are enabled

Test Results:

✅ Unit tests: All 17 test cases passing
✅ Integration tests: New test case passing
✅ Existing tests: All integration tests still passing
✅ Cross-platform: Builds and tests on macOS, compiles for Linux/Windows
Documentation

**Dependencies:**
- Added `k8s.io/mount-utils v0.33.2` for cross-platform mount operations

Future Extension Ready:
When ready to enable unmount functionality:

// Normal uninstall - refuse if mount points exist
system.SafeRemoveAll(path, false, false)

// Force uninstall - unmount and remove
system.SafeRemoveAll(path, true, true)

*Documentation added/planned (if applicable):*

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

<!-- If this is a security issue, please do not discuss on GitHub. Please report any suspected or confirmed security issues to AWS Security https://aws.amazon.com/security/vulnerability-reporting/ -->

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant