fix: normalize path for Windows compatibility #5674
Conversation
|
Please run |
The previous test was testing Go's filepath.Abs/Clean functions instead of testing the Flux code itself. New test: - Actually runs 'flux build kustomization' command - Tests different path formats (relative with/without ./) - Verifies the command produces correct output - Follows the same pattern as existing tests in the file This properly tests that the Flux code normalizes paths correctly, not just that the Go standard library works. Addresses review feedback on PR fluxcd#5674. Signed-off-by: Sibasis Padhi <[email protected]>
cmd/flux/build_kustomization_test.go
Outdated
| args: "build kustomization podinfo --path testdata/build-kustomization/podinfo", | ||
| resultFile: "./testdata/build-kustomization/podinfo-result.yaml", | ||
| assertFunc: "assertGoldenTemplateFile", | ||
| }, | ||
| { | ||
| name: "build with dot-slash relative path", | ||
| args: "build kustomization podinfo --path ./testdata/build-kustomization/podinfo", |
There was a problem hiding this comment.
These paths are too simple and don't test what's changing here. The initial paths from Windows seemed much better in that regard. Maybe we can adapt them to Linux paths?
P.S.: The vibe-coding is wasting a bit of my time 🙃
There was a problem hiding this comment.
Yeah, future is vibe coding. It will get used to soon. Let me recheck & get back.
There was a problem hiding this comment.
Updated. please take a look when possible. The git should be integrated with a code reviewer agent, thats flush out initial issues before going to actual reviewers. That will save time. I am sure CNCF & other teams would be already thinking about it as that is the the vibe coding future. Have fixed it again, please take a look when possible :) . This time my code-assistant said, you will be happy & sorry :). Lets see !! :--)
There was a problem hiding this comment.
Or, are you suggesting to take out the test case all together ?
There was a problem hiding this comment.
Using AI effectively and with other people's time in mind is good. Vibe-coding is not.
Previous tests were too simple. Now testing paths that would actually expose concatenation bugs without normalization: 1. Absolute paths (like Windows C:\path issue) 2. Paths with parent directory (..) that need cleaning 3. Paths with redundant separators (//) that need normalization These tests verify the fix prevents path duplication like: /path/test/path/test/file (without normalization) /path/test/file (with proper normalization) Addresses review feedback on PR fluxcd#5674. Signed-off-by: Sibasis Padhi <[email protected]>
|
Please squash, this PR should have a single commit |
Fixes fluxcd#5673 On Windows, when using absolute paths like C:\path\to\dir, the path could be incorrectly concatenated, resulting in: C:\working\dir\C:\path\to\dir\file This fix applies filepath.Abs() and filepath.Clean() to normalize the path before using it, ensuring absolute paths are handled correctly on all platforms. Changes: - Apply filepath.Abs() to convert relative paths to absolute - Apply filepath.Clean() to remove redundant separators and resolve .. - Add tests for absolute paths, complex paths with .., and paths with redundant separators to verify normalization works correctly The tests use actual 'flux build kustomization' commands with: 1. Absolute paths (prevents concatenation bugs) 2. Paths with parent directory (..) references 3. Paths with redundant separators (//) All tests verify the command produces correct output, ensuring the path normalization fix works as expected. Signed-off-by: Sibasis Padhi <[email protected]>
sibasispadhi
force-pushed
the
fix-windows-path-issue-5673
branch
from
64b3103 to
7dd9fde
Compare
|
thank you for the review, @matheuscscp .done |
|
Successfully created backport PR for |
4 participants
This commit fixes issue #5673 where 'flux build kustomization' fails on Windows when using --path with relative or absolute paths.
The bug manifested as duplicate path components:
C:\path\test\C:\path\test\kustomization.yaml.original
Root cause: The path argument was passed directly without normalization, causing issues in downstream path joining.
Fix: Normalize the path using filepath.Abs() and filepath.Clean() before passing it to the builder. This ensures:
Added unit tests to verify path normalization works correctly on both Linux and Windows platforms.
Fixes #5673