Skip to content

Commit d4a953a

Browse files
committed
Expand relative paths in shell.MakeDirectories
This function needed defined behavior for relative paths. For example, the default log directory for Postgres is just "log" which it interprets relative to the data directory. Issue: PGO-2558
1 parent ef0301e commit d4a953a

File tree

3 files changed

+49
-5
lines changed

3 files changed

+49
-5
lines changed

internal/collector/instance.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package collector
77
import (
88
"context"
99
"fmt"
10-
"path"
1110

1211
corev1 "k8s.io/api/core/v1"
1312

@@ -185,7 +184,7 @@ func startCommand(logDirectories []string, includeLogrotate bool) []string {
185184
if len(logDirectories) != 0 {
186185
for _, logDir := range logDirectories {
187186
mkdirScript = mkdirScript + `
188-
` + shell.MakeDirectories(logDir, path.Join(logDir, "receiver"))
187+
` + shell.MakeDirectories(logDir, "receiver")
189188
}
190189
}
191190

internal/shell/paths.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ func CleanFileName(path string) string {
3636
// exists. It creates every directory leading to path from (but not including)
3737
// base and sets their permissions for Kubernetes, regardless of umask.
3838
//
39+
// Relative paths are expanded relative to base.
40+
//
3941
// See:
4042
// - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/chmod.html
4143
// - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/mkdir.html
@@ -47,8 +49,19 @@ func MakeDirectories(base string, paths ...string) string {
4749
return `test -d ` + QuoteWord(base)
4850
}
4951

50-
allPaths := slices.Clone(paths)
51-
for _, p := range paths {
52+
// Expand each path relative to the base path.
53+
expandedPaths := make([]string, len(paths))
54+
for i, p := range paths {
55+
if filepath.IsAbs(p) {
56+
expandedPaths[i] = p
57+
} else {
58+
expandedPaths[i] = filepath.Join(base, p)
59+
}
60+
}
61+
62+
// Gather parent directories of each path.
63+
allPaths := slices.Clone(expandedPaths)
64+
for _, p := range expandedPaths {
5265
if r, err := filepath.Rel(base, p); err == nil && filepath.IsLocal(r) {
5366
// The result of [filepath.Rel] is a shorter representation of the full path; skip it.
5467
r = filepath.Dir(r)
@@ -72,7 +85,7 @@ func MakeDirectories(base string, paths ...string) string {
7285

7386
return `` +
7487
// Create all the paths and any missing parents.
75-
`mkdir -p ` + strings.Join(QuoteWords(paths...), " ") +
88+
`mkdir -p ` + strings.Join(QuoteWords(expandedPaths...), " ") +
7689

7790
// Try to set the permissions of every path and each parent.
7891
// This swallows the exit status of `chmod` because not all filesystems

internal/shell/paths_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,38 @@ func TestMakeDirectories(t *testing.T) {
9494
})
9595
})
9696

97+
t.Run("Relative", func(t *testing.T) {
98+
assert.Equal(t,
99+
MakeDirectories("/x", "one", "two/three"),
100+
`mkdir -p '/x/one' '/x/two/three' && { chmod 0775 '/x/one' '/x/two/three' '/x/two' || :; }`,
101+
"expected paths relative to base")
102+
103+
assert.Equal(t,
104+
MakeDirectories("/x/y/z", "../one", "./two", "../../../../three"),
105+
`mkdir -p '/x/y/one' '/x/y/z/two' '/three' && { chmod 0775 '/x/y/one' '/x/y/z/two' '/three' || :; }`,
106+
"expected paths relative to base")
107+
108+
script := MakeDirectories("x/y", "../one", "./two", "../../../../three")
109+
assert.Equal(t, script,
110+
`mkdir -p 'x/one' 'x/y/two' '../../three' && { chmod 0775 'x/one' 'x/y/two' '../../three' || :; }`,
111+
"expected paths relative to base")
112+
113+
// Calling `mkdir -p '../..'` works, but run it by ShellCheck as a precaution.
114+
t.Run("ShellCheckPOSIX", func(t *testing.T) {
115+
shellcheck := require.ShellCheck(t)
116+
117+
dir := t.TempDir()
118+
file := filepath.Join(dir, "script.sh")
119+
assert.NilError(t, os.WriteFile(file, []byte(script), 0o600))
120+
121+
// Expect ShellCheck for "sh" to be happy.
122+
// - https://www.shellcheck.net/wiki/SC2148
123+
cmd := exec.CommandContext(t.Context(), shellcheck, "--enable=all", "--shell=sh", file)
124+
output, err := cmd.CombinedOutput()
125+
assert.NilError(t, err, "%q\n%s", cmd.Args, output)
126+
})
127+
})
128+
97129
t.Run("Unrelated", func(t *testing.T) {
98130
assert.Equal(t,
99131
MakeDirectories("/one", "/two/three/four"),

0 commit comments

Comments
 (0)