(fix): fix flaky TestReadDirSymlink

[XuPeng-SH]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #22009

What this PR does / why we need it:

When NewLocalETLFS was called with a rootPath that contained or ended with a symlink directory (e.g., /tmp/ABC/a/b/d/ where d is a symlink to c), the function only used filepath.Abs() which doesn't follow symlinks. This caused path resolution issues later when constructing file paths.

---

PR Type

Bug fix, Tests

---

Description

• Resolve symlinks in rootPath using filepath.EvalSymlinks

• Prevents path resolution issues with symlinked directories

• Add comprehensive test for symlink root path handling

• Test verifies file operations work correctly with symlinked roots

---

Diagram Walkthrough

flowchart LR
  A["NewLocalETLFS called<br/>with symlink rootPath"] -->|filepath.EvalSymlinks| B["Canonical path<br/>resolved"]
  B --> C["LocalETLFS initialized<br/>with real path"]
  C --> D["File operations<br/>work correctly"]

Loading

File Walkthrough

	Relevant files
Bug fix	local_etl_fs.go Add symlink resolution to rootPath initialization                pkg/fileservice/local_etl_fs.go Added filepath.EvalSymlinks() call to resolve symlinks in rootPath Resolves canonical path before initializing LocalETLFS struct Returns error if symlink resolution fails +5/-0
Tests	local_etl_fs_test.go Add test for symlink root path resolution                                pkg/fileservice/local_etl_fs_test.go New test TestLocalETLFSSymlinkRootPath validates symlink handling Creates real directory and symlink, verifies path resolution Tests file listing and reading operations through symlinked root Asserts rootPath matches resolved canonical path +49/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: The new symlink resolution step does not add or reference any auditing/logging for this critical filesystem path decision, and the diff provides no evidence of existing audit coverage. Referred Code // resolve symlinks in the path to get the canonical path rootPath, err = filepath.EvalSymlinks(rootPath) if err != nil { return nil, err }
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Opaque errors: Errors from filepath.EvalSymlinks are returned directly without added context about the operation or path, which may hinder debugging. Referred Code // resolve symlinks in the path to get the canonical path rootPath, err = filepath.EvalSymlinks(rootPath) if err != nil { return nil, err }
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Path validation: The code accepts and follows symlinks without explicit validation against escape from the intended root, which could allow directory traversal if upstream inputs are untrusted. Referred Code // resolve symlinks in the path to get the canonical path rootPath, err = filepath.EvalSymlinks(rootPath) if err != nil { return nil, err }
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Improve test assertion for clarity Improve the test assertion by resolving the symlinkDir path instead of the realDir path to make the test's intent clearer. pkg/fileservice/local_etl_fs_test.go [202-205] // The rootPath should be resolved to the real directory - realDirResolved, err := filepath.EvalSymlinks(realDir) + expectedPath, err := filepath.EvalSymlinks(symlinkDir) assert.Nil(t, err) - assert.Equal(t, realDirResolved, fs.rootPath) + assert.Equal(t, expectedPath, fs.rootPath) Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly improves the test's clarity by resolving the symlink path directly, which more accurately reflects the behavior being tested.	Low
Update