Skip to content

Commit c1bd618

Browse files
committed
improve path handling for cross platform
1 parent bf5c9d9 commit c1bd618

File tree

11 files changed

+122
-65
lines changed

11 files changed

+122
-65
lines changed

api/internal/loader/fileloader.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,9 @@ func newLoaderAtGitClone(
211211
// check for this after cloning repo.
212212
if !root.HasPrefix(repoSpec.CloneDir()) {
213213
_ = cleaner()
214-
return nil, fmt.Errorf("%q refers to directory outside of repo %q", repoSpec.AbsPath(),
215-
repoSpec.CloneDir())
214+
return nil, fmt.Errorf("%q refers to directory outside of repo %q",
215+
filepath.ToSlash(repoSpec.AbsPath()),
216+
filepath.ToSlash(repoSpec.CloneDir().String()))
216217
}
217218
return &FileLoader{
218219
// Clones never allowed to escape root.

api/internal/loader/fileloader_test.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ func TestLoaderLocalScheme(t *testing.T) {
208208
t.Run("file", func(t *testing.T) {
209209
fSys, dir := setupOnDisk(t)
210210
parts := []string{
211-
"ssh:",
211+
"ssh-scheme",
212212
"resource.yaml",
213213
}
214214
require.NoError(t, fSys.Mkdir(dir.Join(parts[0])))
@@ -226,8 +226,9 @@ func TestLoaderLocalScheme(t *testing.T) {
226226
})
227227
t.Run("directory", func(t *testing.T) {
228228
fSys, dir := setupOnDisk(t)
229+
// Use scheme-like name without colon for Windows compatibility
229230
parts := []string{
230-
"https:",
231+
"https-scheme",
231232
"root",
232233
}
233234
require.NoError(t, fSys.MkdirAll(dir.Join(filepath.Join(parts...))))
@@ -379,6 +380,7 @@ func TestNewLoaderAtGitClone(t *testing.T) {
379380
rootURL := "github.com/someOrg/someRepo"
380381
pathInRepo := "foo/base"
381382
url := rootURL + "/" + pathInRepo
383+
// Use absolute path starting with / for in-memory filesystem
382384
coRoot := "/tmp"
383385
fSys := filesys.MakeFsInMemory()
384386
fSys.MkdirAll(coRoot)
@@ -422,6 +424,7 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) {
422424
require := require.New(t)
423425

424426
// Define an overlay-base structure in the file system.
427+
// Use absolute paths for in-memory filesystem
425428
topDir := "/whatever"
426429
cloneRoot := topDir + "/someClone"
427430
fSys := filesys.MakeFsInMemory()
@@ -494,12 +497,14 @@ func TestLoaderDisallowsRemoteBaseExitRepo(t *testing.T) {
494497

495498
_, err = newLoaderAtGitClone(repoSpec, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(repo)))
496499
require.Error(t, err)
497-
require.Contains(t, err.Error(), fmt.Sprintf("%q refers to directory outside of repo %q", base, repo))
500+
// Use filepath.ToSlash to normalize paths for cross-platform comparison
501+
require.Contains(t, err.Error(), fmt.Sprintf("%q refers to directory outside of repo %q", filepath.ToSlash(base), filepath.ToSlash(repo)))
498502
}
499503

500504
func TestLocalLoaderReferencingGitBase(t *testing.T) {
501505
require := require.New(t)
502506

507+
// Use absolute paths for in-memory filesystem
503508
topDir := "/whatever"
504509
cloneRoot := topDir + "/someClone"
505510
fSys := filesys.MakeFsInMemory()
@@ -521,6 +526,7 @@ func TestLocalLoaderReferencingGitBase(t *testing.T) {
521526
func TestRepoDirectCycleDetection(t *testing.T) {
522527
require := require.New(t)
523528

529+
// Use absolute paths for in-memory filesystem
524530
topDir := "/cycles"
525531
cloneRoot := topDir + "/someClone"
526532
fSys := filesys.MakeFsInMemory()
@@ -543,6 +549,7 @@ func TestRepoDirectCycleDetection(t *testing.T) {
543549
func TestRepoIndirectCycleDetection(t *testing.T) {
544550
require := require.New(t)
545551

552+
// Use absolute paths for in-memory filesystem
546553
topDir := "/cycles"
547554
cloneRoot := topDir + "/someClone"
548555
fSys := filesys.MakeFsInMemory()

api/internal/loader/loadrestrictions.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package loader
55

66
import (
77
"fmt"
8+
"path/filepath"
89

910
"sigs.k8s.io/kustomize/kyaml/filesys"
1011
)
@@ -24,7 +25,7 @@ func RestrictionRootOnly(
2425
if !d.HasPrefix(root) {
2526
return "", fmt.Errorf(
2627
"security; file '%s' is not in or below '%s'",
27-
path, root)
28+
filepath.ToSlash(path), filepath.ToSlash(root.String()))
2829
}
2930
return d.Join(f), nil
3031
}

api/internal/loader/loadrestrictions_test.go

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package loader
55

66
import (
7+
"fmt"
78
"path/filepath"
89
"strings"
910
"testing"
@@ -59,9 +60,11 @@ func TestRestrictionRootOnly(t *testing.T) {
5960
if err == nil {
6061
t.Fatal("should have an error")
6162
}
62-
if !strings.Contains(
63-
err.Error(),
64-
"file '/tmp/illegal' is not in or below '/tmp/foo'") {
63+
// Normalize paths to forward slashes for cross-platform comparison
64+
expectedErr := fmt.Sprintf("file '%s' is not in or below '%s'",
65+
filepath.ToSlash(filepath.Join(filesys.Separator+"tmp", "illegal")),
66+
filepath.ToSlash(filepath.Join(filesys.Separator+"tmp", "foo")))
67+
if !strings.Contains(err.Error(), expectedErr) {
6568
t.Fatalf("unexpected err: %s", err)
6669
}
6770
}

api/internal/localizer/util.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,11 @@ func locFilePath(fileURL string) string {
152152
// Raw github urls are the only type of file urls kustomize officially accepts.
153153
// In this case, the path already consists of org, repo, version, and path in repo, in order,
154154
// so we can use it as is.
155-
return filepath.Join(LocalizeDir, u.Hostname(), path)
155+
hostname := u.Hostname()
156+
// On Windows, colons are invalid in file paths. Replace them with hyphens
157+
// to make IPv6 addresses filesystem-safe.
158+
hostname = strings.ReplaceAll(hostname, ":", "-")
159+
return filepath.Join(LocalizeDir, hostname, path)
156160
}
157161

158162
// locRootPath returns the relative localized path of the validated root url rootURL, where the local copy of its repo

api/internal/localizer/util_test.go

Lines changed: 46 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"sigs.k8s.io/kustomize/api/ifc"
1515
"sigs.k8s.io/kustomize/api/internal/git"
1616
"sigs.k8s.io/kustomize/kyaml/filesys"
17+
"sigs.k8s.io/kustomize/kyaml/testutil"
1718
)
1819

1920
func TestDefaultNewDirRepo(t *testing.T) {
@@ -120,7 +121,8 @@ func TestLocFilePathColon(t *testing.T) {
120121

121122
// The colon is special because it was once used as the unix file separator.
122123
const url = "https://[2001:4860:4860::8888]/file.yaml"
123-
const host = "2001:4860:4860::8888"
124+
// On Windows, colons are replaced with hyphens in IPv6 addresses
125+
const host = "2001-4860-4860--8888"
124126
const file = "file.yaml"
125127
req.Equal(simpleJoin(t, LocalizeDir, host, file), locFilePath(url))
126128

@@ -129,8 +131,10 @@ func TestLocFilePathColon(t *testing.T) {
129131

130132
// We check that we can create single directory, meaning ':' not used as file separator.
131133
req.NoError(fSys.Mkdir(targetDir))
132-
_, err := fSys.Create(simpleJoin(t, targetDir, file))
134+
f, err := fSys.Create(simpleJoin(t, targetDir, file))
133135
req.NoError(err)
136+
// Close the file immediately to avoid file locking issues on Windows
137+
req.NoError(f.Close())
134138

135139
// We check that the directory with such name is readable.
136140
files, err := fSys.ReadDir(targetDir)
@@ -139,6 +143,9 @@ func TestLocFilePathColon(t *testing.T) {
139143
}
140144

141145
func TestLocFilePath_SpecialChar(t *testing.T) {
146+
// Skip on Windows as asterisk is invalid in Windows paths
147+
testutil.SkipWindows(t)
148+
142149
req := require.New(t)
143150

144151
// The wild card character is one of the legal uri characters with more meaning
@@ -163,19 +170,26 @@ func TestLocFilePath_SpecialFiles(t *testing.T) {
163170
for name, tFSys := range map[string]struct {
164171
urlPath string
165172
pathDir, pathFile string
173+
skipWindows bool // Skip this test case on Windows
166174
}{
167175
"windows_reserved_name": {
168176
urlPath: "/aux/file",
169177
pathDir: "aux",
170178
pathFile: "file",
171179
},
172180
"hidden_files": {
173-
urlPath: "/.../.file",
174-
pathDir: "...",
175-
pathFile: ".file",
181+
urlPath: "/.../.file",
182+
pathDir: "...",
183+
pathFile: ".file",
184+
skipWindows: true, // Windows treats "..." specially
176185
},
177186
} {
178187
t.Run(name, func(t *testing.T) {
188+
// Skip Windows-incompatible tests
189+
if tFSys.skipWindows {
190+
testutil.SkipWindows(t)
191+
}
192+
179193
req := require.New(t)
180194

181195
expectedPath := simpleJoin(t, LocalizeDir, "host", tFSys.pathDir, tFSys.pathFile)
@@ -304,23 +318,41 @@ func TestLocRootPath_SymlinkPath(t *testing.T) {
304318

305319
func TestCleanedRelativePath(t *testing.T) {
306320
fSys := filesys.MakeFsInMemory()
307-
require.NoError(t, fSys.MkdirAll("/root/test"))
308-
require.NoError(t, fSys.WriteFile("/root/test/file.yaml", []byte("")))
309-
require.NoError(t, fSys.WriteFile("/root/filetwo.yaml", []byte("")))
321+
// Use platform-appropriate root path
322+
rootPath := string(filepath.Separator) + "root"
323+
testPath := filepath.Join(rootPath, "test")
324+
325+
require.NoError(t, fSys.MkdirAll(testPath))
326+
require.NoError(t, fSys.WriteFile(filepath.Join(testPath, "file.yaml"), []byte("")))
327+
require.NoError(t, fSys.WriteFile(filepath.Join(rootPath, "filetwo.yaml"), []byte("")))
310328

311329
// Absolute path is cleaned to relative path
312-
cleanedPath := cleanedRelativePath(fSys, "/root/", "/root/test/file.yaml")
313-
require.Equal(t, "test/file.yaml", cleanedPath)
330+
cleanedPath := cleanedRelativePath(
331+
fSys,
332+
filesys.ConfirmedDir(rootPath+string(filepath.Separator)),
333+
filepath.Join(testPath, "file.yaml"))
334+
require.Equal(t, filepath.Join("test", "file.yaml"), cleanedPath)
314335

315336
// Winding absolute path is cleaned to relative path
316-
cleanedPath = cleanedRelativePath(fSys, "/root/", "/root/test/../filetwo.yaml")
337+
windingPath := filepath.Join(rootPath, "test", "..", "filetwo.yaml")
338+
cleanedPath = cleanedRelativePath(
339+
fSys,
340+
filesys.ConfirmedDir(rootPath+string(filepath.Separator)),
341+
windingPath)
317342
require.Equal(t, "filetwo.yaml", cleanedPath)
318343

319344
// Already clean relative path stays the same
320-
cleanedPath = cleanedRelativePath(fSys, "/root/", "test/file.yaml")
321-
require.Equal(t, "test/file.yaml", cleanedPath)
345+
cleanedPath = cleanedRelativePath(
346+
fSys,
347+
filesys.ConfirmedDir(rootPath+string(filepath.Separator)),
348+
filepath.Join("test", "file.yaml"))
349+
require.Equal(t, filepath.Join("test", "file.yaml"), cleanedPath)
322350

323351
// Winding relative path is cleaned
324-
cleanedPath = cleanedRelativePath(fSys, "/root/", "test/../filetwo.yaml")
352+
windingRelPath := filepath.Join("test", "..", "filetwo.yaml")
353+
cleanedPath = cleanedRelativePath(
354+
fSys,
355+
filesys.ConfirmedDir(rootPath+string(filepath.Separator)),
356+
windingRelPath)
325357
require.Equal(t, "filetwo.yaml", cleanedPath)
326358
}

api/internal/target/kusttarget_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,13 @@ commonLabels:
122122
},
123123
}
124124

125+
// Use platform-appropriate root directory
126+
rootDir := string(filepath.Separator)
125127
kt := makeKustTargetWithRf(
126-
t, th.GetFSys(), "/", provider.NewDefaultDepProvider())
128+
t, th.GetFSys(), rootDir, provider.NewDefaultDepProvider())
127129
for tn, tc := range testCases {
128130
t.Run(tn, func(t *testing.T) {
129-
th.WriteK("/", tc.content)
131+
th.WriteK(rootDir, tc.content)
130132
err := kt.Load()
131133
if tc.errContains != "" {
132134
require.NotNilf(t, err, "expected error containing: `%s`", tc.errContains)

api/krusty/fnplugin_test.go

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,32 +25,6 @@ const (
2525
repoRootDir = "../../"
2626
)
2727

28-
const generateDeploymentDotSh = `#!/bin/sh
29-
30-
cat <<EOF
31-
apiVersion: apps/v1
32-
kind: Deployment
33-
metadata:
34-
name: nginx
35-
labels:
36-
app: nginx
37-
annotations:
38-
tshirt-size: small # this injects the resource reservations
39-
spec:
40-
selector:
41-
matchLabels:
42-
app: nginx
43-
template:
44-
metadata:
45-
labels:
46-
app: nginx
47-
spec:
48-
containers:
49-
- name: nginx
50-
image: nginx
51-
EOF
52-
`
53-
5428
const krmTransformerDotSh = `#!/bin/bash
5529
cat << EOF
5630
apiVersion: v1

api/krusty/originannotation_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,32 @@ import (
1616
"sigs.k8s.io/kustomize/kyaml/filesys"
1717
)
1818

19+
const generateDeploymentDotSh = `#!/bin/sh
20+
21+
cat <<EOF
22+
apiVersion: apps/v1
23+
kind: Deployment
24+
metadata:
25+
name: nginx
26+
labels:
27+
app: nginx
28+
annotations:
29+
tshirt-size: small # this injects the resource reservations
30+
spec:
31+
selector:
32+
matchLabels:
33+
app: nginx
34+
template:
35+
metadata:
36+
labels:
37+
app: nginx
38+
spec:
39+
containers:
40+
- name: nginx
41+
image: nginx
42+
EOF
43+
`
44+
1945
func TestAnnoOriginLocalFiles(t *testing.T) {
2046
th := kusttest_test.MakeHarness(t)
2147
th.WriteF("service.yaml", `

api/resource/origin.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,8 @@ func (origin *Origin) Append(inputPath string) *Origin {
5454
if err == nil {
5555
originCopy.Repo = repoSpec.CloneSpec()
5656
absPath := repoSpec.AbsPath()
57+
// Normalize to forward slashes for cross-platform compatibility
58+
absPath = strings.ReplaceAll(absPath, "\\", "/")
5759
inputPath = absPath[strings.Index(absPath[1:], "/")+1:][1:]
5860
originCopy.Path = ""
5961
originCopy.Ref = repoSpec.Ref

0 commit comments

Comments
 (0)