From 4621079f713098ca28b98dc6571f7458646f5829 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 17:32:02 +0100 Subject: [PATCH 1/7] Read template from [fs.FS] instead of assuming local paths This removes the need to pre-materialize built-in templates. --- libs/jsonschema/schema.go | 9 +++- libs/jsonschema/schema_test.go | 7 +++ libs/template/config.go | 6 +-- libs/template/config_test.go | 28 ++++++------ libs/template/file.go | 21 ++++++--- libs/template/file_test.go | 12 ++---- libs/template/helpers_test.go | 16 +++---- libs/template/materialize.go | 71 +++---------------------------- libs/template/materialize_test.go | 3 +- libs/template/renderer.go | 57 ++++++++++++++----------- libs/template/renderer_test.go | 58 ++++++++----------------- 11 files changed, 120 insertions(+), 168 deletions(-) diff --git a/libs/jsonschema/schema.go b/libs/jsonschema/schema.go index 7690ec2f76..b9c3fb08cd 100644 --- a/libs/jsonschema/schema.go +++ b/libs/jsonschema/schema.go @@ -3,7 +3,9 @@ package jsonschema import ( "encoding/json" "fmt" + "io/fs" "os" + "path/filepath" "regexp" "slices" @@ -255,7 +257,12 @@ func (schema *Schema) validate() error { } func Load(path string) (*Schema, error) { - b, err := os.ReadFile(path) + dir, file := filepath.Split(path) + return LoadFS(os.DirFS(dir), file) +} + +func LoadFS(fsys fs.FS, path string) (*Schema, error) { + b, err := fs.ReadFile(fsys, path) if err != nil { return nil, err } diff --git a/libs/jsonschema/schema_test.go b/libs/jsonschema/schema_test.go index cf1f127672..d66868bb25 100644 --- a/libs/jsonschema/schema_test.go +++ b/libs/jsonschema/schema_test.go @@ -1,6 +1,7 @@ package jsonschema import ( + "os" "testing" "github.com/stretchr/testify/assert" @@ -305,3 +306,9 @@ func TestValidateSchemaSkippedPropertiesHaveDefaults(t *testing.T) { err = s.validate() assert.NoError(t, err) } + +func TestSchema_LoadFS(t *testing.T) { + fsys := os.DirFS("./testdata/schema-load-int") + _, err := LoadFS(fsys, "schema-valid.json") + assert.NoError(t, err) +} diff --git a/libs/template/config.go b/libs/template/config.go index 5470aefeb6..8ebc4d8434 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "io/fs" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/jsonschema" @@ -28,9 +29,8 @@ type config struct { schema *jsonschema.Schema } -func newConfig(ctx context.Context, schemaPath string) (*config, error) { - // Read config schema - schema, err := jsonschema.Load(schemaPath) +func newConfig(ctx context.Context, templateRoot fs.FS, schemaPath string) (*config, error) { + schema, err := jsonschema.LoadFS(templateRoot, schemaPath) if err != nil { return nil, err } diff --git a/libs/template/config_test.go b/libs/template/config_test.go index ab9dbeb5f9..46eddb3813 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -3,6 +3,7 @@ package template import ( "context" "fmt" + "os" "path/filepath" "testing" "text/template" @@ -16,7 +17,7 @@ func TestTemplateConfigAssignValuesFromFile(t *testing.T) { testDir := "./testdata/config-assign-from-file" ctx := context.Background() - c, err := newConfig(ctx, filepath.Join(testDir, "schema.json")) + c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) err = c.assignValuesFromFile(filepath.Join(testDir, "config.json")) @@ -32,7 +33,7 @@ func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *te testDir := "./testdata/config-assign-from-file" ctx := context.Background() - c, err := newConfig(ctx, filepath.Join(testDir, "schema.json")) + c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) c.values = map[string]any{ @@ -52,7 +53,7 @@ func TestTemplateConfigAssignValuesFromFileForInvalidIntegerValue(t *testing.T) testDir := "./testdata/config-assign-from-file-invalid-int" ctx := context.Background() - c, err := newConfig(ctx, filepath.Join(testDir, "schema.json")) + c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) err = c.assignValuesFromFile(filepath.Join(testDir, "config.json")) @@ -63,7 +64,7 @@ func TestTemplateConfigAssignValuesFromFileFiltersPropertiesNotInTheSchema(t *te testDir := "./testdata/config-assign-from-file-unknown-property" ctx := context.Background() - c, err := newConfig(ctx, filepath.Join(testDir, "schema.json")) + c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) err = c.assignValuesFromFile(filepath.Join(testDir, "config.json")) @@ -78,10 +79,10 @@ func TestTemplateConfigAssignValuesFromDefaultValues(t *testing.T) { testDir := "./testdata/config-assign-from-default-value" ctx := context.Background() - c, err := newConfig(ctx, filepath.Join(testDir, "schema.json")) + c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) - r, err := newRenderer(ctx, nil, nil, "./testdata/empty/template", "./testdata/empty/library", t.TempDir()) + r, err := newRenderer(ctx, nil, nil, os.DirFS("."), "./testdata/empty/template", "./testdata/empty/library", t.TempDir()) require.NoError(t, err) err = c.assignDefaultValues(r) @@ -97,10 +98,10 @@ func TestTemplateConfigAssignValuesFromTemplatedDefaultValues(t *testing.T) { testDir := "./testdata/config-assign-from-templated-default-value" ctx := context.Background() - c, err := newConfig(ctx, filepath.Join(testDir, "schema.json")) + c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) - r, err := newRenderer(ctx, nil, nil, filepath.Join(testDir, "template/template"), filepath.Join(testDir, "template/library"), t.TempDir()) + r, err := newRenderer(ctx, nil, nil, os.DirFS("."), filepath.Join(testDir, "template/template"), filepath.Join(testDir, "template/library"), t.TempDir()) require.NoError(t, err) // Note: only the string value is templated. @@ -116,7 +117,7 @@ func TestTemplateConfigAssignValuesFromTemplatedDefaultValues(t *testing.T) { func TestTemplateConfigValidateValuesDefined(t *testing.T) { ctx := context.Background() - c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json") + c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json") require.NoError(t, err) c.values = map[string]any{ @@ -131,7 +132,7 @@ func TestTemplateConfigValidateValuesDefined(t *testing.T) { func TestTemplateConfigValidateTypeForValidConfig(t *testing.T) { ctx := context.Background() - c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json") + c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json") require.NoError(t, err) c.values = map[string]any{ @@ -147,7 +148,7 @@ func TestTemplateConfigValidateTypeForValidConfig(t *testing.T) { func TestTemplateConfigValidateTypeForUnknownField(t *testing.T) { ctx := context.Background() - c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json") + c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json") require.NoError(t, err) c.values = map[string]any{ @@ -164,7 +165,7 @@ func TestTemplateConfigValidateTypeForUnknownField(t *testing.T) { func TestTemplateConfigValidateTypeForInvalidType(t *testing.T) { ctx := context.Background() - c, err := newConfig(ctx, "testdata/config-test-schema/test-schema.json") + c, err := newConfig(ctx, os.DirFS("testdata/config-test-schema"), "test-schema.json") require.NoError(t, err) c.values = map[string]any{ @@ -271,7 +272,8 @@ func TestTemplateEnumValidation(t *testing.T) { } func TestTemplateSchemaErrorsWithEmptyDescription(t *testing.T) { - _, err := newConfig(context.Background(), "./testdata/config-test-schema/invalid-test-schema.json") + ctx := context.Background() + _, err := newConfig(ctx, os.DirFS("./testdata/config-test-schema"), "invalid-test-schema.json") assert.EqualError(t, err, "template property property-without-description is missing a description") } diff --git a/libs/template/file.go b/libs/template/file.go index aafb1acfae..5492ebeb4d 100644 --- a/libs/template/file.go +++ b/libs/template/file.go @@ -6,8 +6,7 @@ import ( "io/fs" "os" "path/filepath" - - "github.com/databricks/cli/libs/filer" + "slices" ) // Interface representing a file to be materialized from a template into a project @@ -19,6 +18,10 @@ type file interface { // Write file to disk at the destination path. PersistToDisk() error + + // contents returns the file contents as a byte slice. + // This is used for testing purposes. + contents() ([]byte, error) } type destinationPath struct { @@ -46,8 +49,8 @@ type copyFile struct { dstPath *destinationPath - // Filer rooted at template root. Used to read srcPath. - srcFiler filer.Filer + // [fs.FS] rooted at template root. Used to read srcPath. + srcFS fs.FS // Relative path from template root for file to be copied. srcPath string @@ -63,7 +66,7 @@ func (f *copyFile) PersistToDisk() error { if err != nil { return err } - srcFile, err := f.srcFiler.Read(f.ctx, f.srcPath) + srcFile, err := f.srcFS.Open(f.srcPath) if err != nil { return err } @@ -77,6 +80,10 @@ func (f *copyFile) PersistToDisk() error { return err } +func (f *copyFile) contents() ([]byte, error) { + return fs.ReadFile(f.srcFS, f.srcPath) +} + type inMemoryFile struct { dstPath *destinationPath @@ -99,3 +106,7 @@ func (f *inMemoryFile) PersistToDisk() error { } return os.WriteFile(path, f.content, f.perm) } + +func (f *inMemoryFile) contents() ([]byte, error) { + return slices.Clone(f.content), nil +} diff --git a/libs/template/file_test.go b/libs/template/file_test.go index 85938895ef..e1bd54564c 100644 --- a/libs/template/file_test.go +++ b/libs/template/file_test.go @@ -8,7 +8,6 @@ import ( "runtime" "testing" - "github.com/databricks/cli/libs/filer" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -33,10 +32,7 @@ func testInMemoryFile(t *testing.T, perm fs.FileMode) { 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) + err := os.WriteFile(filepath.Join(tmpDir, "source"), []byte("qwerty"), perm) require.NoError(t, err) f := ©File{ @@ -45,9 +41,9 @@ func testCopyFile(t *testing.T, perm fs.FileMode) { root: tmpDir, relPath: "a/b/c", }, - perm: perm, - srcPath: "source", - srcFiler: templateFiler, + perm: perm, + srcPath: "source", + srcFS: os.DirFS(tmpDir), } err = f.PersistToDisk() assert.NoError(t, err) diff --git a/libs/template/helpers_test.go b/libs/template/helpers_test.go index 8cc7b928ed..8a779eccb8 100644 --- a/libs/template/helpers_test.go +++ b/libs/template/helpers_test.go @@ -22,7 +22,7 @@ func TestTemplatePrintStringWithoutProcessing(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/print-without-processing/template", "./testdata/print-without-processing/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/print-without-processing/template", "./testdata/print-without-processing/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -39,7 +39,7 @@ func TestTemplateRegexpCompileFunction(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/regexp-compile/template", "./testdata/regexp-compile/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/regexp-compile/template", "./testdata/regexp-compile/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -57,7 +57,7 @@ func TestTemplateRandIntFunction(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/random-int/template", "./testdata/random-int/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/random-int/template", "./testdata/random-int/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -75,7 +75,7 @@ func TestTemplateUuidFunction(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/uuid/template", "./testdata/uuid/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/uuid/template", "./testdata/uuid/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -92,7 +92,7 @@ func TestTemplateUrlFunction(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/urlparse-function/template", "./testdata/urlparse-function/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/urlparse-function/template", "./testdata/urlparse-function/library", tmpDir) require.NoError(t, err) @@ -109,7 +109,7 @@ func TestTemplateMapPairFunction(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/map-pair/template", "./testdata/map-pair/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/map-pair/template", "./testdata/map-pair/library", tmpDir) require.NoError(t, err) @@ -132,7 +132,7 @@ func TestWorkspaceHost(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, w) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/workspace-host/template", "./testdata/map-pair/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/workspace-host/template", "./testdata/map-pair/library", tmpDir) require.NoError(t, err) @@ -157,7 +157,7 @@ func TestWorkspaceHostNotConfigured(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, w) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/workspace-host/template", "./testdata/map-pair/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/workspace-host/template", "./testdata/map-pair/library", tmpDir) assert.NoError(t, err) diff --git a/libs/template/materialize.go b/libs/template/materialize.go index d824bf381a..e26a599b39 100644 --- a/libs/template/materialize.go +++ b/libs/template/materialize.go @@ -6,9 +6,6 @@ import ( "errors" "fmt" "io/fs" - "os" - "path" - "path/filepath" "github.com/databricks/cli/libs/cmdio" ) @@ -28,28 +25,12 @@ var builtinTemplates embed.FS // configFilePath: file path containing user defined config values // templateRoot: root of the template definition // outputDir: root of directory where to initialize the template -func Materialize(ctx context.Context, configFilePath, templateRoot, outputDir string) error { - // Use a temporary directory in case any builtin templates like default-python are used - tempDir, err := os.MkdirTemp("", "templates") - defer os.RemoveAll(tempDir) - if err != nil { - return err +func Materialize(ctx context.Context, configFilePath string, templateRoot fs.FS, outputDir string) error { + if _, err := fs.Stat(templateRoot, schemaFileName); errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("not a bundle template: expected to find a template schema file at %s", schemaFileName) } - templateRoot, err = prepareBuiltinTemplates(templateRoot, tempDir) - if err != nil { - return err - } - - templatePath := filepath.Join(templateRoot, templateDirName) - libraryPath := filepath.Join(templateRoot, libraryDirName) - schemaPath := filepath.Join(templateRoot, schemaFileName) - helpers := loadHelpers(ctx) - if _, err := os.Stat(schemaPath); errors.Is(err, fs.ErrNotExist) { - return fmt.Errorf("not a bundle template: expected to find a template schema file at %s", schemaPath) - } - - config, err := newConfig(ctx, schemaPath) + config, err := newConfig(ctx, templateRoot, schemaFileName) if err != nil { return err } @@ -62,7 +43,8 @@ func Materialize(ctx context.Context, configFilePath, templateRoot, outputDir st } } - r, err := newRenderer(ctx, config.values, helpers, templatePath, libraryPath, outputDir) + helpers := loadHelpers(ctx) + r, err := newRenderer(ctx, config.values, helpers, templateRoot, templateDirName, libraryDirName, outputDir) if err != nil { return err } @@ -111,44 +93,3 @@ func Materialize(ctx context.Context, configFilePath, templateRoot, outputDir st } return nil } - -// If the given templateRoot matches -func prepareBuiltinTemplates(templateRoot string, tempDir string) (string, error) { - // Check that `templateRoot` is a clean basename, i.e. `some_path` and not `./some_path` or "." - // Return early if that's not the case. - if templateRoot == "." || path.Base(templateRoot) != templateRoot { - return templateRoot, nil - } - - _, err := fs.Stat(builtinTemplates, path.Join("templates", templateRoot)) - if err != nil { - // The given path doesn't appear to be using out built-in templates - return templateRoot, nil - } - - // We have a built-in template with the same name as templateRoot! - // Now we need to make a fully copy of the builtin templates to a real file system - // since template.Parse() doesn't support embed.FS. - err = fs.WalkDir(builtinTemplates, "templates", func(path string, entry fs.DirEntry, err error) error { - if err != nil { - return err - } - - targetPath := filepath.Join(tempDir, path) - if entry.IsDir() { - return os.Mkdir(targetPath, 0755) - } else { - content, err := fs.ReadFile(builtinTemplates, path) - if err != nil { - return err - } - return os.WriteFile(targetPath, content, 0644) - } - }) - - if err != nil { - return "", err - } - - return filepath.Join(tempDir, "templates", templateRoot), nil -} diff --git a/libs/template/materialize_test.go b/libs/template/materialize_test.go index b4be3fe980..a79de77597 100644 --- a/libs/template/materialize_test.go +++ b/libs/template/materialize_test.go @@ -3,6 +3,7 @@ package template import ( "context" "fmt" + "os" "path/filepath" "testing" @@ -19,6 +20,6 @@ func TestMaterializeForNonTemplateDirectory(t *testing.T) { ctx := root.SetWorkspaceClient(context.Background(), w) // Try to materialize a non-template directory. - err = Materialize(ctx, "", tmpDir, "") + err = Materialize(ctx, "", os.DirFS(tmpDir), "") assert.EqualError(t, err, fmt.Sprintf("not a bundle template: expected to find a template schema file at %s", filepath.Join(tmpDir, schemaFileName))) } diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 827f30133b..b35f150af2 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -8,14 +8,12 @@ import ( "io/fs" "os" "path" - "path/filepath" "regexp" "slices" "sort" "strings" "text/template" - "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go/logger" ) @@ -52,32 +50,43 @@ type renderer struct { // do not match any glob patterns from this list skipPatterns []string - // Filer rooted at template root. The file tree from this root is walked to + // [fs.FS] for the template root. The file tree from this root is walked to // generate the project - templateFiler filer.Filer + templateRoot fs.FS // Root directory for the project instantiated from the template instanceRoot string } -func newRenderer(ctx context.Context, config map[string]any, helpers template.FuncMap, templateRoot, libraryRoot, instanceRoot string) (*renderer, error) { +func newRenderer( + ctx context.Context, + config map[string]any, + helpers template.FuncMap, + templateFS fs.FS, + templateDir string, + libraryDir string, + instanceRoot string, +) (*renderer, error) { // Initialize new template, with helper functions loaded tmpl := template.New("").Funcs(helpers) - // Load user defined associated templates from the library root - libraryGlob := filepath.Join(libraryRoot, "*") - matches, err := filepath.Glob(libraryGlob) + // Find user-defined templates in the library directory + matches, err := fs.Glob(templateFS, path.Join(libraryDir, "*")) if err != nil { return nil, err } + + // Parse user-defined templates. + // Note: we do not call [ParseFS] with the glob directly because + // it returns an error if no files match the pattern. if len(matches) != 0 { - tmpl, err = tmpl.ParseFiles(matches...) + tmpl, err = tmpl.ParseFS(templateFS, matches...) if err != nil { return nil, err } } - templateFiler, err := filer.NewLocalClient(templateRoot) + fsys, err := fs.Sub(templateFS, path.Clean(templateDir)) if err != nil { return nil, err } @@ -85,13 +94,13 @@ func newRenderer(ctx context.Context, config map[string]any, helpers template.Fu ctx = log.NewContext(ctx, log.GetLogger(ctx).With("action", "initialize-template")) return &renderer{ - ctx: ctx, - config: config, - baseTemplate: tmpl, - files: make([]file, 0), - skipPatterns: make([]string, 0), - templateFiler: templateFiler, - instanceRoot: instanceRoot, + ctx: ctx, + config: config, + baseTemplate: tmpl, + files: make([]file, 0), + skipPatterns: make([]string, 0), + templateRoot: fsys, + instanceRoot: instanceRoot, }, nil } @@ -141,7 +150,7 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { func (r *renderer) computeFile(relPathTemplate string) (file, error) { // read file permissions - info, err := r.templateFiler.Stat(r.ctx, relPathTemplate) + info, err := fs.Stat(r.templateRoot, relPathTemplate) if err != nil { return nil, err } @@ -161,10 +170,10 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { root: r.instanceRoot, relPath: relPath, }, - perm: perm, - ctx: r.ctx, - srcPath: relPathTemplate, - srcFiler: r.templateFiler, + perm: perm, + ctx: r.ctx, + srcFS: r.templateRoot, + srcPath: relPathTemplate, }, nil } else { // Trim the .tmpl suffix from file name, if specified in the template @@ -173,7 +182,7 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { } // read template file's content - templateReader, err := r.templateFiler.Read(r.ctx, relPathTemplate) + templateReader, err := r.templateRoot.Open(relPathTemplate) if err != nil { return nil, err } @@ -263,7 +272,7 @@ func (r *renderer) walk() error { // // 2. For directories: They are appended to a slice, which acts as a queue // allowing BFS traversal of the template file tree - entries, err := r.templateFiler.ReadDir(r.ctx, currentDirectory) + entries, err := fs.ReadDir(r.templateRoot, currentDirectory) if err != nil { return err } diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 92133c5fea..9b8861e786 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -3,9 +3,9 @@ package template import ( "context" "fmt" - "io" "io/fs" "os" + "path" "path/filepath" "runtime" "strings" @@ -41,9 +41,8 @@ func assertFilePermissions(t *testing.T, path string, perm fs.FileMode) { func assertBuiltinTemplateValid(t *testing.T, template string, settings map[string]any, target string, isServicePrincipal bool, build bool, tempDir string) { ctx := context.Background() - templatePath, err := prepareBuiltinTemplates(template, tempDir) + templateFS, err := fs.Sub(builtinTemplates, path.Join("templates", template)) require.NoError(t, err) - libraryPath := filepath.Join(templatePath, "library") w := &databricks.WorkspaceClient{ Config: &workspaceConfig.Config{Host: "https://myhost.com"}, @@ -58,7 +57,7 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri ctx = root.SetWorkspaceClient(ctx, w) helpers := loadHelpers(ctx) - renderer, err := newRenderer(ctx, settings, helpers, templatePath, libraryPath, tempDir) + renderer, err := newRenderer(ctx, settings, helpers, templateFS, templateDirName, libraryDirName, tempDir) require.NoError(t, err) // Evaluate template @@ -67,7 +66,7 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri err = renderer.persistToDisk() require.NoError(t, err) - b, err := bundle.Load(ctx, filepath.Join(tempDir, "template", "my_project")) + b, err := bundle.Load(ctx, filepath.Join(tempDir, "my_project")) require.NoError(t, err) diags := bundle.Apply(ctx, b, phases.LoadNamedTarget(target)) require.NoError(t, diags.Error()) @@ -96,18 +95,6 @@ func assertBuiltinTemplateValid(t *testing.T, template string, settings map[stri } } -func TestPrepareBuiltInTemplatesWithRelativePaths(t *testing.T) { - // CWD should not be resolved as a built in template - dir, err := prepareBuiltinTemplates(".", t.TempDir()) - assert.NoError(t, err) - assert.Equal(t, ".", dir) - - // relative path should not be resolved as a built in template - dir, err = prepareBuiltinTemplates("./default-python", t.TempDir()) - assert.NoError(t, err) - assert.Equal(t, "./default-python", dir) -} - func TestBuiltinPythonTemplateValid(t *testing.T) { // Test option combinations options := []string{"yes", "no"} @@ -194,7 +181,7 @@ func TestRendererWithAssociatedTemplateInLibrary(t *testing.T) { ctx := context.Background() ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/email/template", "./testdata/email/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/email/template", "./testdata/email/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -381,7 +368,7 @@ func TestRendererWalk(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/walk/template", "./testdata/walk/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/walk/template", "./testdata/walk/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -392,18 +379,9 @@ func TestRendererWalk(t *testing.T) { 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") - } + b, err := f.contents() + require.NoError(t, err) + return strings.Trim(string(b), "\r\n") } require.FailNow(t, "file is absent: "+path) return "" @@ -422,7 +400,7 @@ func TestRendererFailFunction(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/fail/template", "./testdata/fail/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/fail/template", "./testdata/fail/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -435,7 +413,7 @@ func TestRendererSkipsDirsEagerly(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/skip-dir-eagerly/template", "./testdata/skip-dir-eagerly/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/skip-dir-eagerly/template", "./testdata/skip-dir-eagerly/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -452,7 +430,7 @@ func TestRendererSkipAllFilesInCurrentDirectory(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/skip-all-files-in-cwd/template", "./testdata/skip-all-files-in-cwd/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/skip-all-files-in-cwd/template", "./testdata/skip-all-files-in-cwd/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -475,7 +453,7 @@ func TestRendererSkipPatternsAreRelativeToFileDirectory(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/skip-is-relative/template", "./testdata/skip-is-relative/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/skip-is-relative/template", "./testdata/skip-is-relative/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -493,7 +471,7 @@ func TestRendererSkip(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/skip/template", "./testdata/skip/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/skip/template", "./testdata/skip/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -525,7 +503,7 @@ func TestRendererReadsPermissionsBits(t *testing.T) { ctx = root.SetWorkspaceClient(ctx, nil) helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/executable-bit-read/template", "./testdata/executable-bit-read/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/executable-bit-read/template", "./testdata/executable-bit-read/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -615,7 +593,7 @@ func TestRendererNonTemplatesAreCreatedAsCopyFiles(t *testing.T) { tmpDir := t.TempDir() helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, nil, helpers, "./testdata/copy-file-walk/template", "./testdata/copy-file-walk/library", tmpDir) + r, err := newRenderer(ctx, nil, helpers, os.DirFS("."), "./testdata/copy-file-walk/template", "./testdata/copy-file-walk/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -635,7 +613,7 @@ func TestRendererFileTreeRendering(t *testing.T) { r, err := newRenderer(ctx, map[string]any{ "dir_name": "my_directory", "file_name": "my_file", - }, helpers, "./testdata/file-tree-rendering/template", "./testdata/file-tree-rendering/library", tmpDir) + }, helpers, os.DirFS("."), "./testdata/file-tree-rendering/template", "./testdata/file-tree-rendering/library", tmpDir) require.NoError(t, err) err = r.walk() @@ -668,7 +646,7 @@ func TestRendererSubTemplateInPath(t *testing.T) { testutil.Touch(t, filepath.Join(templateDir, "template/{{template `dir_name`}}/{{template `file_name`}}")) tmpDir := t.TempDir() - r, err := newRenderer(ctx, nil, nil, filepath.Join(templateDir, "template"), filepath.Join(templateDir, "library"), tmpDir) + r, err := newRenderer(ctx, nil, nil, os.DirFS(templateDir), "template", "library", tmpDir) require.NoError(t, err) err = r.walk() From e2174f5870af423e055dfd59d5967da5f553390c Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 20:35:36 +0100 Subject: [PATCH 2/7] Make init command aware of built-in templates --- cmd/bundle/init.go | 35 ++++++++++++++++++++++++-- libs/template/builtin.go | 47 +++++++++++++++++++++++++++++++++++ libs/template/builtin_test.go | 28 +++++++++++++++++++++ libs/template/materialize.go | 4 --- 4 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 libs/template/builtin.go create mode 100644 libs/template/builtin_test.go diff --git a/cmd/bundle/init.go b/cmd/bundle/init.go index 7f2c0efc58..d31a702a19 100644 --- a/cmd/bundle/init.go +++ b/cmd/bundle/init.go @@ -3,6 +3,7 @@ package bundle import ( "errors" "fmt" + "io/fs" "os" "path/filepath" "slices" @@ -109,6 +110,24 @@ func getUrlForNativeTemplate(name string) string { return "" } +func getFsForNativeTemplate(name string) (fs.FS, error) { + builtin, err := template.Builtin() + if err != nil { + return nil, err + } + + // If this is a built-in template, the return value will be non-nil. + var templateFS fs.FS + for _, entry := range builtin { + if entry.Name == name { + templateFS = entry.FS + break + } + } + + return templateFS, nil +} + func isRepoUrl(url string) bool { result := false for _, prefix := range gitUrlPrefixes { @@ -198,9 +217,20 @@ See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more inf if templateDir != "" { return errors.New("--template-dir can only be used with a Git repository URL") } + + templateFS, err := getFsForNativeTemplate(templatePath) + if err != nil { + return err + } + + // If this is not a built-in template, then it must be a local file system path. + if templateFS == nil { + templateFS = os.DirFS(templatePath) + } + // skip downloading the repo because input arg is not a URL. We assume // it's a path on the local file system in that case - return template.Materialize(ctx, configFile, templatePath, outputDir) + return template.Materialize(ctx, configFile, templateFS, outputDir) } // Create a temporary directory with the name of the repository. The '*' @@ -224,7 +254,8 @@ See https://docs.databricks.com/en/dev-tools/bundles/templates.html for more inf // Clean up downloaded repository once the template is materialized. defer os.RemoveAll(repoDir) - return template.Materialize(ctx, configFile, filepath.Join(repoDir, templateDir), outputDir) + templateFS := os.DirFS(filepath.Join(repoDir, templateDir)) + return template.Materialize(ctx, configFile, templateFS, outputDir) } return cmd } diff --git a/libs/template/builtin.go b/libs/template/builtin.go new file mode 100644 index 0000000000..dcb3a88582 --- /dev/null +++ b/libs/template/builtin.go @@ -0,0 +1,47 @@ +package template + +import ( + "embed" + "io/fs" +) + +//go:embed all:templates +var builtinTemplates embed.FS + +// BuiltinTemplate represents a template that is built into the CLI. +type BuiltinTemplate struct { + Name string + FS fs.FS +} + +// Builtin returns the list of all built-in templates. +func Builtin() ([]BuiltinTemplate, error) { + templates, err := fs.Sub(builtinTemplates, "templates") + if err != nil { + return nil, err + } + + entries, err := fs.ReadDir(templates, ".") + if err != nil { + return nil, err + } + + var out []BuiltinTemplate + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + templateFS, err := fs.Sub(templates, entry.Name()) + if err != nil { + return nil, err + } + + out = append(out, BuiltinTemplate{ + Name: entry.Name(), + FS: templateFS, + }) + } + + return out, nil +} diff --git a/libs/template/builtin_test.go b/libs/template/builtin_test.go new file mode 100644 index 0000000000..504e0acca2 --- /dev/null +++ b/libs/template/builtin_test.go @@ -0,0 +1,28 @@ +package template + +import ( + "io/fs" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuiltin(t *testing.T) { + out, err := Builtin() + require.NoError(t, err) + assert.Len(t, out, 3) + + // Confirm names. + assert.Equal(t, "dbt-sql", out[0].Name) + assert.Equal(t, "default-python", out[1].Name) + assert.Equal(t, "default-sql", out[2].Name) + + // Confirm that the filesystems work. + _, err = fs.Stat(out[0].FS, `template/{{.project_name}}/dbt_project.yml.tmpl`) + assert.NoError(t, err) + _, err = fs.Stat(out[1].FS, `template/{{.project_name}}/tests/main_test.py.tmpl`) + assert.NoError(t, err) + _, err = fs.Stat(out[2].FS, `template/{{.project_name}}/src/orders_daily.sql.tmpl`) + assert.NoError(t, err) +} diff --git a/libs/template/materialize.go b/libs/template/materialize.go index e26a599b39..c24519efd6 100644 --- a/libs/template/materialize.go +++ b/libs/template/materialize.go @@ -2,7 +2,6 @@ package template import ( "context" - "embed" "errors" "fmt" "io/fs" @@ -14,9 +13,6 @@ const libraryDirName = "library" const templateDirName = "template" const schemaFileName = "databricks_template_schema.json" -//go:embed all:templates -var builtinTemplates embed.FS - // This function materializes the input templates as a project, using user defined // configurations. // Parameters: From c4d565a98a2a58fc52c97a1d3b9fa84b3e1269bc Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 20:43:12 +0100 Subject: [PATCH 3/7] Rename --- libs/template/config.go | 4 ++-- libs/template/materialize.go | 8 ++++---- libs/template/renderer.go | 17 ++++++++--------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/libs/template/config.go b/libs/template/config.go index 8ebc4d8434..8e7695b915 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -29,8 +29,8 @@ type config struct { schema *jsonschema.Schema } -func newConfig(ctx context.Context, templateRoot fs.FS, schemaPath string) (*config, error) { - schema, err := jsonschema.LoadFS(templateRoot, schemaPath) +func newConfig(ctx context.Context, templateFS fs.FS, schemaPath string) (*config, error) { + schema, err := jsonschema.LoadFS(templateFS, schemaPath) if err != nil { return nil, err } diff --git a/libs/template/materialize.go b/libs/template/materialize.go index c24519efd6..15b1b3c3cf 100644 --- a/libs/template/materialize.go +++ b/libs/template/materialize.go @@ -21,12 +21,12 @@ const schemaFileName = "databricks_template_schema.json" // configFilePath: file path containing user defined config values // templateRoot: root of the template definition // outputDir: root of directory where to initialize the template -func Materialize(ctx context.Context, configFilePath string, templateRoot fs.FS, outputDir string) error { - if _, err := fs.Stat(templateRoot, schemaFileName); errors.Is(err, fs.ErrNotExist) { +func Materialize(ctx context.Context, configFilePath string, templateFS fs.FS, outputDir string) error { + if _, err := fs.Stat(templateFS, schemaFileName); errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("not a bundle template: expected to find a template schema file at %s", schemaFileName) } - config, err := newConfig(ctx, templateRoot, schemaFileName) + config, err := newConfig(ctx, templateFS, schemaFileName) if err != nil { return err } @@ -40,7 +40,7 @@ func Materialize(ctx context.Context, configFilePath string, templateRoot fs.FS, } helpers := loadHelpers(ctx) - r, err := newRenderer(ctx, config.values, helpers, templateRoot, templateDirName, libraryDirName, outputDir) + r, err := newRenderer(ctx, config.values, helpers, templateFS, templateDirName, libraryDirName, outputDir) if err != nil { return err } diff --git a/libs/template/renderer.go b/libs/template/renderer.go index b35f150af2..bc86503993 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -50,9 +50,8 @@ type renderer struct { // do not match any glob patterns from this list skipPatterns []string - // [fs.FS] for the template root. The file tree from this root is walked to - // generate the project - templateRoot fs.FS + // [fs.FS] that holds the template's file tree. + srcFS fs.FS // Root directory for the project instantiated from the template instanceRoot string @@ -86,7 +85,7 @@ func newRenderer( } } - fsys, err := fs.Sub(templateFS, path.Clean(templateDir)) + srcFS, err := fs.Sub(templateFS, path.Clean(templateDir)) if err != nil { return nil, err } @@ -99,7 +98,7 @@ func newRenderer( baseTemplate: tmpl, files: make([]file, 0), skipPatterns: make([]string, 0), - templateRoot: fsys, + srcFS: srcFS, instanceRoot: instanceRoot, }, nil } @@ -150,7 +149,7 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { func (r *renderer) computeFile(relPathTemplate string) (file, error) { // read file permissions - info, err := fs.Stat(r.templateRoot, relPathTemplate) + info, err := fs.Stat(r.srcFS, relPathTemplate) if err != nil { return nil, err } @@ -172,7 +171,7 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { }, perm: perm, ctx: r.ctx, - srcFS: r.templateRoot, + srcFS: r.srcFS, srcPath: relPathTemplate, }, nil } else { @@ -182,7 +181,7 @@ func (r *renderer) computeFile(relPathTemplate string) (file, error) { } // read template file's content - templateReader, err := r.templateRoot.Open(relPathTemplate) + templateReader, err := r.srcFS.Open(relPathTemplate) if err != nil { return nil, err } @@ -272,7 +271,7 @@ func (r *renderer) walk() error { // // 2. For directories: They are appended to a slice, which acts as a queue // allowing BFS traversal of the template file tree - entries, err := fs.ReadDir(r.templateRoot, currentDirectory) + entries, err := fs.ReadDir(r.srcFS, currentDirectory) if err != nil { return err } From e5bd9d981cdfe5d155fa4c5f8f1fd23d7fb8b41a Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 20:55:05 +0100 Subject: [PATCH 4/7] Adapt integration test helper --- internal/bundle/helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/bundle/helpers.go b/internal/bundle/helpers.go index 8f1a866f65..9740061ece 100644 --- a/internal/bundle/helpers.go +++ b/internal/bundle/helpers.go @@ -42,7 +42,7 @@ func initTestTemplateWithBundleRoot(t *testing.T, ctx context.Context, templateN cmd := cmdio.NewIO(flags.OutputJSON, strings.NewReader(""), os.Stdout, os.Stderr, "", "bundles") ctx = cmdio.InContext(ctx, cmd) - err = template.Materialize(ctx, configFilePath, templateRoot, bundleRoot) + err = template.Materialize(ctx, configFilePath, os.DirFS(templateRoot), bundleRoot) return bundleRoot, err } From 1d7862da7b0ff4909bd7c5ea5e194a0dc7b0d829 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 20:55:15 +0100 Subject: [PATCH 5/7] Update test assertion --- libs/template/materialize_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/template/materialize_test.go b/libs/template/materialize_test.go index a79de77597..dc510a30d4 100644 --- a/libs/template/materialize_test.go +++ b/libs/template/materialize_test.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "path/filepath" "testing" "github.com/databricks/cli/cmd/root" @@ -21,5 +20,5 @@ func TestMaterializeForNonTemplateDirectory(t *testing.T) { // Try to materialize a non-template directory. err = Materialize(ctx, "", os.DirFS(tmpDir), "") - assert.EqualError(t, err, fmt.Sprintf("not a bundle template: expected to find a template schema file at %s", filepath.Join(tmpDir, schemaFileName))) + assert.EqualError(t, err, fmt.Sprintf("not a bundle template: expected to find a template schema file at %s", schemaFileName)) } From fcb798850a68de4e00e671c1ca8d39042195f73e Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 21:13:10 +0100 Subject: [PATCH 6/7] Fix Windows test --- libs/template/config_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 46eddb3813..49d3423e2f 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "path" "path/filepath" "testing" "text/template" @@ -101,7 +102,7 @@ func TestTemplateConfigAssignValuesFromTemplatedDefaultValues(t *testing.T) { c, err := newConfig(ctx, os.DirFS(testDir), "schema.json") require.NoError(t, err) - r, err := newRenderer(ctx, nil, nil, os.DirFS("."), filepath.Join(testDir, "template/template"), filepath.Join(testDir, "template/library"), t.TempDir()) + r, err := newRenderer(ctx, nil, nil, os.DirFS("."), path.Join(testDir, "template/template"), path.Join(testDir, "template/library"), t.TempDir()) require.NoError(t, err) // Note: only the string value is templated. From fd5a2b9785a4ad2b8499d1a3c5c821e5d80555a9 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 18 Nov 2024 22:29:25 +0100 Subject: [PATCH 7/7] Update comment --- libs/template/materialize.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/template/materialize.go b/libs/template/materialize.go index 15b1b3c3cf..0163eb7d22 100644 --- a/libs/template/materialize.go +++ b/libs/template/materialize.go @@ -19,7 +19,7 @@ const schemaFileName = "databricks_template_schema.json" // // ctx: context containing a cmdio object. This is used to prompt the user // configFilePath: file path containing user defined config values -// templateRoot: root of the template definition +// templateFS: root of the template definition // outputDir: root of directory where to initialize the template func Materialize(ctx context.Context, configFilePath string, templateFS fs.FS, outputDir string) error { if _, err := fs.Stat(templateFS, schemaFileName); errors.Is(err, fs.ErrNotExist) {