Skip to content

Commit 23f05f5

Browse files
authored
Set the write bit for files written during template initialization (#2068)
## Changes This used to work because the permission bits for built-in templates were hardcoded to 0644 for files and 0755 for directories. As of #1912 (and the PRs it depends on), built-in templates are no longer pre-materialized to a temporary directory and read directly from the embedded filesystem. This built-in filesystem returns 0444 as the permission bits for the files it contains. These bits are carried over to the destination filesystem. This change updates template materialization to always set the owner's write bit. It doesn't really make sense to write read-only files and expect users to work with these files in a VCS (note: Git only stores the executable bit). The regression shipped as part of v0.235.0 and will be fixed as of v0.238.0. ## Tests Unit tests.
1 parent 0289bec commit 23f05f5

File tree

4 files changed

+60
-19
lines changed

4 files changed

+60
-19
lines changed

internal/testutil/file.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"os"
55
"path/filepath"
66

7+
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89
)
910

@@ -52,3 +53,31 @@ func ReadFile(t TestingT, path string) string {
5253

5354
return string(b)
5455
}
56+
57+
// StatFile returns the file info for a file.
58+
func StatFile(t TestingT, path string) os.FileInfo {
59+
fi, err := os.Stat(path)
60+
require.NoError(t, err)
61+
62+
return fi
63+
}
64+
65+
// AssertFileContents asserts that the file at path has the expected content.
66+
func AssertFileContents(t TestingT, path, expected string) bool {
67+
actual := ReadFile(t, path)
68+
return assert.Equal(t, expected, actual)
69+
}
70+
71+
// AssertFilePermissions asserts that the file at path has the expected permissions.
72+
func AssertFilePermissions(t TestingT, path string, expected os.FileMode) bool {
73+
fi := StatFile(t, path)
74+
assert.False(t, fi.Mode().IsDir(), "expected a file, got a directory")
75+
return assert.Equal(t, expected, fi.Mode().Perm(), "expected 0%o, got 0%o", expected, fi.Mode().Perm())
76+
}
77+
78+
// AssertDirPermissions asserts that the file at path has the expected permissions.
79+
func AssertDirPermissions(t TestingT, path string, expected os.FileMode) bool {
80+
fi := StatFile(t, path)
81+
assert.True(t, fi.Mode().IsDir(), "expected a directory, got a file")
82+
return assert.Equal(t, expected, fi.Mode().Perm(), "expected 0%o, got 0%o", expected, fi.Mode().Perm())
83+
}

libs/template/file_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"runtime"
99
"testing"
1010

11+
"github.com/databricks/cli/internal/testutil"
1112
"github.com/databricks/cli/libs/filer"
1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
@@ -27,8 +28,8 @@ func testInMemoryFile(t *testing.T, ctx context.Context, perm fs.FileMode) {
2728
err = f.Write(ctx, out)
2829
assert.NoError(t, err)
2930

30-
assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123")
31-
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
31+
testutil.AssertFileContents(t, filepath.Join(tmpDir, "a/b/c"), "123")
32+
testutil.AssertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
3233
}
3334

3435
func testCopyFile(t *testing.T, ctx context.Context, perm fs.FileMode) {
@@ -48,8 +49,8 @@ func testCopyFile(t *testing.T, ctx context.Context, perm fs.FileMode) {
4849
err = f.Write(ctx, out)
4950
assert.NoError(t, err)
5051

51-
assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty")
52-
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
52+
testutil.AssertFileContents(t, filepath.Join(tmpDir, "source"), "qwerty")
53+
testutil.AssertFilePermissions(t, filepath.Join(tmpDir, "source"), perm)
5354
}
5455

5556
func TestTemplateInMemoryFilePersistToDisk(t *testing.T) {

libs/template/renderer.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,10 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) {
150150
}
151151
perm := info.Mode().Perm()
152152

153+
// Always include the write bit for the owner of the file.
154+
// It does not make sense to have a file that is not writable by the owner.
155+
perm |= 0o200
156+
153157
// Execute relative path template to get destination path for the file
154158
relPath, err := r.executeTemplate(relPathTemplate)
155159
if err != nil {

libs/template/renderer_test.go

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -27,16 +27,19 @@ import (
2727
"github.com/stretchr/testify/require"
2828
)
2929

30-
func assertFileContent(t *testing.T, path, content string) {
31-
b, err := os.ReadFile(path)
32-
require.NoError(t, err)
33-
assert.Equal(t, content, string(b))
34-
}
30+
var (
31+
defaultFilePermissions fs.FileMode
32+
defaultDirPermissions fs.FileMode
33+
)
3534

36-
func assertFilePermissions(t *testing.T, path string, perm fs.FileMode) {
37-
info, err := os.Stat(path)
38-
require.NoError(t, err)
39-
assert.Equal(t, perm, info.Mode().Perm())
35+
func init() {
36+
if runtime.GOOS == "windows" {
37+
defaultFilePermissions = fs.FileMode(0o666)
38+
defaultDirPermissions = fs.FileMode(0o777)
39+
} else {
40+
defaultFilePermissions = fs.FileMode(0o644)
41+
defaultDirPermissions = fs.FileMode(0o755)
42+
}
4043
}
4144

4245
func assertBuiltinTemplateValid(t *testing.T, template string, settings map[string]any, target string, isServicePrincipal, build bool, tempDir string) {
@@ -69,6 +72,10 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri
6972
err = renderer.persistToDisk(ctx, out)
7073
require.NoError(t, err)
7174

75+
// Verify permissions on file and directory
76+
testutil.AssertFilePermissions(t, filepath.Join(tempDir, "my_project/README.md"), defaultFilePermissions)
77+
testutil.AssertDirPermissions(t, filepath.Join(tempDir, "my_project/resources"), defaultDirPermissions)
78+
7279
b, err := bundle.Load(ctx, filepath.Join(tempDir, "my_project"))
7380
require.NoError(t, err)
7481
diags := bundle.Apply(ctx, b, phases.LoadNamedTarget(target))
@@ -347,10 +354,10 @@ func TestRendererPersistToDisk(t *testing.T) {
347354
assert.NoFileExists(t, filepath.Join(tmpDir, "a", "b", "c"))
348355
assert.NoFileExists(t, filepath.Join(tmpDir, "mno"))
349356

350-
assertFileContent(t, filepath.Join(tmpDir, "a", "b", "d"), "123")
351-
assertFilePermissions(t, filepath.Join(tmpDir, "a", "b", "d"), 0o444)
352-
assertFileContent(t, filepath.Join(tmpDir, "mmnn"), "456")
353-
assertFilePermissions(t, filepath.Join(tmpDir, "mmnn"), 0o444)
357+
testutil.AssertFileContents(t, filepath.Join(tmpDir, "a/b/d"), "123")
358+
testutil.AssertFilePermissions(t, filepath.Join(tmpDir, "a/b/d"), fs.FileMode(0o444))
359+
testutil.AssertFileContents(t, filepath.Join(tmpDir, "mmnn"), "456")
360+
testutil.AssertFilePermissions(t, filepath.Join(tmpDir, "mmnn"), fs.FileMode(0o444))
354361
}
355362

356363
func TestRendererWalk(t *testing.T) {
@@ -617,8 +624,8 @@ func TestRendererFileTreeRendering(t *testing.T) {
617624
require.NoError(t, err)
618625

619626
// Assert files and directories are correctly materialized.
620-
assert.DirExists(t, filepath.Join(tmpDir, "my_directory"))
621-
assert.FileExists(t, filepath.Join(tmpDir, "my_directory", "my_file"))
627+
testutil.AssertDirPermissions(t, filepath.Join(tmpDir, "my_directory"), defaultDirPermissions)
628+
testutil.AssertFilePermissions(t, filepath.Join(tmpDir, "my_directory", "my_file"), defaultFilePermissions)
622629
}
623630

624631
func TestRendererSubTemplateInPath(t *testing.T) {

0 commit comments

Comments
 (0)