Only treat files with .tmpl extension as templates (#594)

## Changes
In a world before this PR, all files would be treated as `go text
templates`, making the content in these files quake in fear since they
would be executed (as a template).

This PR makes it so that only files with the `.tmpl` extension are
understood to be templates. This is useful for avoiding ambiguity in
cases like where a binary file could be interpreted as a go text
template otherwise.

In order to do so, we introduce the `copyFile` struct which does a copy
of the source file from the template without loading it into memory.

## Tests
Unit tests
This commit is contained in:
shreyas-goenka 2023-08-01 15:43:27 +02:00 committed by GitHub
parent bb415ce6bb
commit fc8729d162
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 370 additions and 166 deletions

101
libs/template/file.go Normal file
View File

@ -0,0 +1,101 @@
package template
import (
"context"
"io"
"io/fs"
"os"
"path/filepath"
"github.com/databricks/cli/libs/filer"
)
// Interface representing a file to be materialized from a template into a project
// instance
type file interface {
// Destination path for file. This is where the file will be created when
// PersistToDisk is called.
DstPath() *destinationPath
// Write file to disk at the destination path.
PersistToDisk() error
}
type destinationPath struct {
// Root path for the project instance. This path uses the system's default
// file separator. For example /foo/bar on Unix and C:\foo\bar on windows
root string
// Unix like file path relative to the "root" of the instantiated project. Is used to
// evaluate whether the file should be skipped by comparing it to a list of
// skip glob patterns.
relPath string
}
// Absolute path of the file, in the os native format. For example /foo/bar on
// Unix and C:\foo\bar on windows
func (f *destinationPath) absPath() string {
return filepath.Join(f.root, filepath.FromSlash(f.relPath))
}
type copyFile struct {
ctx context.Context
// Permissions bits for the destination file
perm fs.FileMode
dstPath *destinationPath
// Filer rooted at template root. Used to read srcPath.
srcFiler filer.Filer
// Relative path from template root for file to be copied.
srcPath string
}
func (f *copyFile) DstPath() *destinationPath {
return f.dstPath
}
func (f *copyFile) PersistToDisk() error {
path := f.DstPath().absPath()
err := os.MkdirAll(filepath.Dir(path), 0755)
if err != nil {
return err
}
srcFile, err := f.srcFiler.Read(f.ctx, f.srcPath)
if err != nil {
return err
}
defer srcFile.Close()
dstFile, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, f.perm)
if err != nil {
return err
}
defer dstFile.Close()
_, err = io.Copy(dstFile, srcFile)
return err
}
type inMemoryFile struct {
dstPath *destinationPath
content []byte
// Permissions bits for the destination file
perm fs.FileMode
}
func (f *inMemoryFile) DstPath() *destinationPath {
return f.dstPath
}
func (f *inMemoryFile) PersistToDisk() error {
path := f.DstPath().absPath()
err := os.MkdirAll(filepath.Dir(path), 0755)
if err != nil {
return err
}
return os.WriteFile(path, f.content, f.perm)
}

111
libs/template/file_test.go Normal file
View File

@ -0,0 +1,111 @@
package template
import (
"context"
"io/fs"
"os"
"path/filepath"
"runtime"
"testing"
"github.com/databricks/cli/libs/filer"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func testInMemoryFile(t *testing.T, perm fs.FileMode) {
tmpDir := t.TempDir()
f := &inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "a/b/c",
},
perm: perm,
content: []byte("123"),
}
err := f.PersistToDisk()
assert.NoError(t, err)
assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123")
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
}
func testCopyFile(t *testing.T, perm fs.FileMode) {
tmpDir := t.TempDir()
templateFiler, err := filer.NewLocalClient(tmpDir)
require.NoError(t, err)
err = os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), perm)
require.NoError(t, err)
f := &copyFile{
ctx: context.Background(),
dstPath: &destinationPath{
root: tmpDir,
relPath: "a/b/c",
},
perm: perm,
srcPath: "source",
srcFiler: templateFiler,
}
err = f.PersistToDisk()
assert.NoError(t, err)
assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "qwerty")
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), perm)
}
func TestTemplateFileDestinationPath(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
f := &destinationPath{
root: `a/b/c`,
relPath: "d/e",
}
assert.Equal(t, `a/b/c/d/e`, f.absPath())
}
func TestTemplateFileDestinationPathForWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
}
f := &destinationPath{
root: `c:\a\b\c`,
relPath: "d/e",
}
assert.Equal(t, `c:\a\b\c\d\e`, f.absPath())
}
func TestTemplateInMemoryFilePersistToDisk(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
testInMemoryFile(t, 0755)
}
func TestTemplateInMemoryFilePersistToDiskForWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
}
// we have separate tests for windows because of differences in valid
// fs.FileMode values we can use for different operating systems.
testInMemoryFile(t, 0666)
}
func TestTemplateCopyFilePersistToDisk(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
testCopyFile(t, 0644)
}
func TestTemplateCopyFilePersistToDiskForWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
}
// we have separate tests for windows because of differences in valid
// fs.FileMode values we can use for different operating systems.
testCopyFile(t, 0666)
}

View File

@ -20,7 +20,7 @@ func TestTemplatePrintStringWithoutProcessing(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, r.files, 1)
cleanContent := strings.Trim(string(r.files[0].content), "\n\r")
cleanContent := strings.Trim(string(r.files[0].(*inMemoryFile).content), "\n\r")
assert.Equal(t, `{{ fail "abc" }}`, cleanContent)
}
@ -35,7 +35,7 @@ func TestTemplateRegexpCompileFunction(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, r.files, 1)
content := string(r.files[0].content)
content := string(r.files[0].(*inMemoryFile).content)
assert.Contains(t, content, "0:food")
assert.Contains(t, content, "1:fool")
}
@ -52,5 +52,5 @@ func TestTemplateUrlFunction(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, r.files, 1)
assert.Equal(t, "https://www.databricks.com", string(r.files[0].content))
assert.Equal(t, "https://www.databricks.com", string(r.files[0].(*inMemoryFile).content))
}

View File

@ -5,7 +5,6 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"os"
"path"
"path/filepath"
@ -18,32 +17,7 @@ import (
"golang.org/x/exp/slices"
)
type inMemoryFile struct {
// Root path for the project instance. This path uses the system's default
// file separator. For example /foo/bar on Unix and C:\foo\bar on windows
root string
// Unix like relPath for the file (using '/' as the separator). This path
// is relative to the root. Using unix like relative paths enables skip patterns
// to work across both windows and unix based operating systems.
relPath string
content []byte
perm fs.FileMode
}
func (f *inMemoryFile) fullPath() string {
return filepath.Join(f.root, filepath.FromSlash(f.relPath))
}
func (f *inMemoryFile) persistToDisk() error {
path := f.fullPath()
err := os.MkdirAll(filepath.Dir(path), 0755)
if err != nil {
return err
}
return os.WriteFile(path, f.content, f.perm)
}
const templateExtension = ".tmpl"
// Renders a databricks template as a project
type renderer struct {
@ -60,7 +34,7 @@ type renderer struct {
baseTemplate *template.Template
// List of in memory files generated from template
files []*inMemoryFile
files []file
// Glob patterns for files and directories to skip. There are three possible
// outcomes for skip:
@ -111,7 +85,7 @@ func newRenderer(ctx context.Context, config map[string]any, templateRoot, libra
ctx: ctx,
config: config,
baseTemplate: tmpl,
files: make([]*inMemoryFile, 0),
files: make([]file, 0),
skipPatterns: make([]string, 0),
templateFiler: templateFiler,
instanceRoot: instanceRoot,
@ -142,17 +116,7 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) {
return result.String(), nil
}
func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) {
// read template file contents
templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate)
if err != nil {
return nil, err
}
contentTemplate, err := io.ReadAll(templateReader)
if err != nil {
return nil, err
}
func (r *renderer) computeFile(relPathTemplate string) (file, error) {
// read file permissions
info, err := r.templateFiler.Stat(r.ctx, relPathTemplate)
if err != nil {
@ -160,7 +124,33 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) {
}
perm := info.Mode().Perm()
// If file name does not specify the `.tmpl` extension, then it is copied
// over as is, without treating it as a template
if !strings.HasSuffix(relPathTemplate, templateExtension) {
return &copyFile{
dstPath: &destinationPath{
root: r.instanceRoot,
relPath: relPathTemplate,
},
perm: perm,
ctx: r.ctx,
srcPath: relPathTemplate,
srcFiler: r.templateFiler,
}, nil
}
// read template file's content
templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate)
if err != nil {
return nil, err
}
defer templateReader.Close()
// execute the contents of the file as a template
contentTemplate, err := io.ReadAll(templateReader)
if err != nil {
return nil, err
}
content, err := r.executeTemplate(string(contentTemplate))
// Capture errors caused by the "fail" helper function
if target := (&ErrFail{}); errors.As(err, target) {
@ -171,16 +161,19 @@ func (r *renderer) computeFile(relPathTemplate string) (*inMemoryFile, error) {
}
// Execute relative path template to get materialized path for the file
relPathTemplate = strings.TrimSuffix(relPathTemplate, templateExtension)
relPath, err := r.executeTemplate(relPathTemplate)
if err != nil {
return nil, err
}
return &inMemoryFile{
dstPath: &destinationPath{
root: r.instanceRoot,
relPath: relPath,
content: []byte(content),
},
perm: perm,
content: []byte(content),
}, nil
}
@ -206,11 +199,11 @@ func (r *renderer) walk() error {
if err != nil {
return err
}
isSkipped, err := r.isSkipped(instanceDirectory)
match, err := isSkipped(instanceDirectory, r.skipPatterns)
if err != nil {
return err
}
if isSkipped {
if match {
logger.Infof(r.ctx, "skipping directory: %s", instanceDirectory)
continue
}
@ -255,7 +248,7 @@ func (r *renderer) walk() error {
if err != nil {
return err
}
logger.Infof(r.ctx, "added file to list of in memory files: %s", f.relPath)
logger.Infof(r.ctx, "added file to list of possible project files: %s", f.DstPath().relPath)
r.files = append(r.files, f)
}
@ -266,14 +259,14 @@ func (r *renderer) walk() error {
func (r *renderer) persistToDisk() error {
// Accumulate files which we will persist, skipping files whose path matches
// any of the skip patterns
filesToPersist := make([]*inMemoryFile, 0)
filesToPersist := make([]file, 0)
for _, file := range r.files {
isSkipped, err := r.isSkipped(file.relPath)
match, err := isSkipped(file.DstPath().relPath, r.skipPatterns)
if err != nil {
return err
}
if isSkipped {
log.Infof(r.ctx, "skipping file: %s", file.relPath)
if match {
log.Infof(r.ctx, "skipping file: %s", file.DstPath())
continue
}
filesToPersist = append(filesToPersist, file)
@ -281,7 +274,7 @@ func (r *renderer) persistToDisk() error {
// Assert no conflicting files exist
for _, file := range filesToPersist {
path := file.fullPath()
path := file.DstPath().absPath()
_, err := os.Stat(path)
if err == nil {
return fmt.Errorf("failed to persist to disk, conflict with existing file: %s", path)
@ -293,7 +286,7 @@ func (r *renderer) persistToDisk() error {
// Persist files to disk
for _, file := range filesToPersist {
err := file.persistToDisk()
err := file.PersistToDisk()
if err != nil {
return err
}
@ -301,8 +294,8 @@ func (r *renderer) persistToDisk() error {
return nil
}
func (r *renderer) isSkipped(filePath string) (bool, error) {
for _, pattern := range r.skipPatterns {
func isSkipped(filePath string, patterns []string) (bool, error) {
for _, pattern := range patterns {
isMatch, err := path.Match(pattern, filePath)
if err != nil {
return false, err

View File

@ -3,6 +3,7 @@ package template
import (
"context"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
@ -89,59 +90,58 @@ My email is {{template "email"}}
}
func TestRendererIsSkipped(t *testing.T) {
r := renderer{
skipPatterns: []string{"a*", "*yz", "def", "a/b/*"},
}
skipPatterns := []string{"a*", "*yz", "def", "a/b/*"}
// skipped paths
isSkipped, err := r.isSkipped("abc")
match, err := isSkipped("abc", skipPatterns)
require.NoError(t, err)
assert.True(t, isSkipped)
assert.True(t, match)
isSkipped, err = r.isSkipped("abcd")
match, err = isSkipped("abcd", skipPatterns)
require.NoError(t, err)
assert.True(t, isSkipped)
assert.True(t, match)
isSkipped, err = r.isSkipped("a")
match, err = isSkipped("a", skipPatterns)
require.NoError(t, err)
assert.True(t, isSkipped)
assert.True(t, match)
isSkipped, err = r.isSkipped("xxyz")
match, err = isSkipped("xxyz", skipPatterns)
require.NoError(t, err)
assert.True(t, isSkipped)
assert.True(t, match)
isSkipped, err = r.isSkipped("yz")
match, err = isSkipped("yz", skipPatterns)
require.NoError(t, err)
assert.True(t, isSkipped)
assert.True(t, match)
isSkipped, err = r.isSkipped("a/b/c")
match, err = isSkipped("a/b/c", skipPatterns)
require.NoError(t, err)
assert.True(t, isSkipped)
assert.True(t, match)
// NOT skipped paths
isSkipped, err = r.isSkipped(".")
match, err = isSkipped(".", skipPatterns)
require.NoError(t, err)
assert.False(t, isSkipped)
assert.False(t, match)
isSkipped, err = r.isSkipped("y")
match, err = isSkipped("y", skipPatterns)
require.NoError(t, err)
assert.False(t, isSkipped)
assert.False(t, match)
isSkipped, err = r.isSkipped("z")
match, err = isSkipped("z", skipPatterns)
require.NoError(t, err)
assert.False(t, isSkipped)
assert.False(t, match)
isSkipped, err = r.isSkipped("defg")
match, err = isSkipped("defg", skipPatterns)
require.NoError(t, err)
assert.False(t, isSkipped)
assert.False(t, match)
isSkipped, err = r.isSkipped("cat")
match, err = isSkipped("cat", skipPatterns)
require.NoError(t, err)
assert.False(t, isSkipped)
assert.False(t, match)
isSkipped, err = r.isSkipped("a/b/c/d")
match, err = isSkipped("a/b/c/d", skipPatterns)
require.NoError(t, err)
assert.False(t, isSkipped)
assert.False(t, match)
}
func TestRendererPersistToDisk(t *testing.T) {
@ -152,30 +152,38 @@ func TestRendererPersistToDisk(t *testing.T) {
ctx: ctx,
instanceRoot: tmpDir,
skipPatterns: []string{"a/b/c", "mn*"},
files: []*inMemoryFile{
{
files: []file{
&inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "a/b/c",
content: nil,
perm: 0444,
},
{
perm: 0444,
content: nil,
},
&inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "mno",
content: nil,
perm: 0444,
},
{
perm: 0444,
content: nil,
},
&inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "a/b/d",
content: []byte("123"),
perm: 0444,
},
{
perm: 0444,
content: []byte("123"),
},
&inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "mmnn",
content: []byte("456"),
},
perm: 0444,
content: []byte("456"),
},
},
}
@ -204,8 +212,20 @@ func TestRendererWalk(t *testing.T) {
getContent := func(r *renderer, path string) string {
for _, f := range r.files {
if f.relPath == path {
return strings.Trim(string(f.content), "\r\n")
if f.DstPath().relPath != path {
continue
}
switch v := f.(type) {
case *inMemoryFile:
return strings.Trim(string(v.content), "\r\n")
case *copyFile:
r, err := r.templateFiler.Read(context.Background(), v.srcPath)
require.NoError(t, err)
b, err := io.ReadAll(r)
require.NoError(t, err)
return strings.Trim(string(b), "\r\n")
default:
require.FailNow(t, "execution should not reach here")
}
}
require.FailNow(t, "file is absent: "+path)
@ -241,7 +261,7 @@ func TestRendererSkipsDirsEagerly(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, r.files, 1)
content := string(r.files[0].content)
content := string(r.files[0].(*inMemoryFile).content)
assert.Equal(t, "I should be the only file created", strings.Trim(content, "\r\n"))
}
@ -309,55 +329,6 @@ func TestRendererSkip(t *testing.T) {
assert.NoFileExists(t, filepath.Join(tmpDir, "dir2/file6"))
}
func TestRendererInMemoryFileFullPathForWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
}
f := &inMemoryFile{
root: `c:\a\b\c`,
relPath: "d/e",
}
assert.Equal(t, `c:\a\b\c\d\e`, f.fullPath())
}
func TestRendererInMemoryFilePersistToDiskSetsExecutableBit(t *testing.T) {
if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
t.SkipNow()
}
tmpDir := t.TempDir()
f := &inMemoryFile{
root: tmpDir,
relPath: "a/b/c",
content: []byte("123"),
perm: 0755,
}
err := f.persistToDisk()
assert.NoError(t, err)
assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123")
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0755)
}
func TestRendererInMemoryFilePersistToDiskForWindows(t *testing.T) {
if runtime.GOOS != "windows" {
t.SkipNow()
}
tmpDir := t.TempDir()
f := &inMemoryFile{
root: tmpDir,
relPath: "a/b/c",
content: []byte("123"),
perm: 0666,
}
err := f.persistToDisk()
assert.NoError(t, err)
assertFileContent(t, filepath.Join(tmpDir, "a/b/c"), "123")
assertFilePermissions(t, filepath.Join(tmpDir, "a/b/c"), 0666)
}
func TestRendererReadsPermissionsBits(t *testing.T) {
if runtime.GOOS != "linux" && runtime.GOOS != "darwin" {
t.SkipNow()
@ -373,8 +344,16 @@ func TestRendererReadsPermissionsBits(t *testing.T) {
getPermissions := func(r *renderer, path string) fs.FileMode {
for _, f := range r.files {
if f.relPath == path {
return f.perm
if f.DstPath().relPath != path {
continue
}
switch v := f.(type) {
case *inMemoryFile:
return v.perm
case *copyFile:
return v.perm
default:
require.FailNow(t, "execution should not reach here")
}
}
require.FailNow(t, "file is absent: "+path)
@ -396,12 +375,14 @@ func TestRendererErrorOnConflictingFile(t *testing.T) {
r := renderer{
skipPatterns: []string{},
files: []*inMemoryFile{
{
files: []file{
&inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "a",
content: []byte("123"),
},
perm: 0444,
content: []byte("123"),
},
},
}
@ -421,12 +402,14 @@ func TestRendererNoErrorOnConflictingFileIfSkipped(t *testing.T) {
r := renderer{
ctx: ctx,
skipPatterns: []string{"a"},
files: []*inMemoryFile{
{
files: []file{
&inMemoryFile{
dstPath: &destinationPath{
root: tmpDir,
relPath: "a",
content: []byte("123"),
},
perm: 0444,
content: []byte("123"),
},
},
}
@ -436,3 +419,18 @@ func TestRendererNoErrorOnConflictingFileIfSkipped(t *testing.T) {
assert.NoError(t, err)
assert.Len(t, r.files, 1)
}
func TestRendererNonTemplatesAreCreatedAsCopyFiles(t *testing.T) {
ctx := context.Background()
tmpDir := t.TempDir()
r, err := newRenderer(ctx, nil, "./testdata/copy-file-walk/template", "./testdata/copy-file-walk/library", tmpDir)
require.NoError(t, err)
err = r.walk()
assert.NoError(t, err)
assert.Len(t, r.files, 1)
assert.Equal(t, r.files[0].(*copyFile).srcPath, "not-a-template")
assert.Equal(t, r.files[0].DstPath().absPath(), filepath.Join(tmpDir, "not-a-template"))
}

View File

@ -0,0 +1 @@
abc