From d33c09a694f031c67ea07f3808d16059e366e27b Mon Sep 17 00:00:00 2001 From: monalisa Date: Tue, 12 Sep 2023 11:28:30 +0200 Subject: [PATCH 01/10] Error when unknown keys are encounters during template execution --- libs/template/renderer.go | 6 ++++++ libs/template/renderer_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index f674ea0fb90..adfee71979e 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -98,6 +98,12 @@ func newRenderer(ctx context.Context, config map[string]any, helpers template.Fu func (r *renderer) executeTemplate(templateDefinition string) (string, error) { // Create copy of base template so as to not overwrite it tmpl, err := r.baseTemplate.Clone() + + // The template execution will error instead of printing on unknown + // map keys if the "missingkey=error" option is set. + // We do this here instead of doing this once for r.baseTemplate because + // the Template.Clone() method does not clone options. + tmpl = tmpl.Option("missingkey=error") if err != nil { return "", err } diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index 21dd1e4fa1c..c541d3b512b 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -189,6 +189,22 @@ My email is {{template "email"}} assert.Contains(t, statement, `My email is hrithik.roshan@databricks.com`) } +func TestRendererExecuteTemplateWithUnknownProperty(t *testing.T) { + templateText := `{{.does_not_exist}}` + + r := renderer{ + config: map[string]any{ + "Material": "wool", + "count": 1, + "Animal": "sheep", + }, + baseTemplate: template.New("base"), + } + + _, err := r.executeTemplate(templateText) + assert.ErrorContains(t, err, "map has no entry for key \"does_not_exist\"") +} + func TestRendererIsSkipped(t *testing.T) { skipPatterns := []string{"a*", "*yz", "def", "a/b/*"} From d5f4664f47e7dec2af716ef967dadbc6809d9f32 Mon Sep 17 00:00:00 2001 From: monalisa Date: Tue, 12 Sep 2023 11:41:48 +0200 Subject: [PATCH 02/10] added end to end test --- internal/init_test.go | 13 +++++++++++++ .../databricks_template_schema.json | 8 ++++++++ .../init/field-does-not-exist/template/bar.tmpl | 3 +++ 3 files changed, 24 insertions(+) create mode 100644 internal/init_test.go create mode 100644 internal/testdata/init/field-does-not-exist/databricks_template_schema.json create mode 100644 internal/testdata/init/field-does-not-exist/template/bar.tmpl diff --git a/internal/init_test.go b/internal/init_test.go new file mode 100644 index 00000000000..c55aee06f77 --- /dev/null +++ b/internal/init_test.go @@ -0,0 +1,13 @@ +package internal + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBundleInitErrorOnUnknownFields(t *testing.T) { + tmpDir := t.TempDir() + _, _, err := RequireErrorRun(t, "bundle", "init", "./testdata/init/field-does-not-exist", "--output-dir", tmpDir) + assert.EqualError(t, err, `failed to compute file content for bar.tmpl. template: :2:2: executing "" at <.does_not_exist>: map has no entry for key "does_not_exist"`) +} diff --git a/internal/testdata/init/field-does-not-exist/databricks_template_schema.json b/internal/testdata/init/field-does-not-exist/databricks_template_schema.json new file mode 100644 index 00000000000..c37fc0895c2 --- /dev/null +++ b/internal/testdata/init/field-does-not-exist/databricks_template_schema.json @@ -0,0 +1,8 @@ +{ + "properties": { + "foo": { + "type": "string", + "default": "abc" + } + } +} diff --git a/internal/testdata/init/field-does-not-exist/template/bar.tmpl b/internal/testdata/init/field-does-not-exist/template/bar.tmpl new file mode 100644 index 00000000000..95f8d250137 --- /dev/null +++ b/internal/testdata/init/field-does-not-exist/template/bar.tmpl @@ -0,0 +1,3 @@ +{{.foo}} +{{.does_not_exist}} +hello, world From 0b8d7db68b4ea8bf41c3791d2e50103bf41e6ff8 Mon Sep 17 00:00:00 2001 From: monalisa Date: Tue, 12 Sep 2023 11:43:13 +0200 Subject: [PATCH 03/10] - --- libs/template/renderer_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index c541d3b512b..f6943abfd11 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -193,11 +193,7 @@ func TestRendererExecuteTemplateWithUnknownProperty(t *testing.T) { templateText := `{{.does_not_exist}}` r := renderer{ - config: map[string]any{ - "Material": "wool", - "count": 1, - "Animal": "sheep", - }, + config: map[string]any{}, baseTemplate: template.New("base"), } From 7e6c4e8480a1987e668e38af6607c8e923cc26e2 Mon Sep 17 00:00:00 2001 From: monalisa Date: Tue, 12 Sep 2023 13:55:59 +0200 Subject: [PATCH 04/10] skip e2e test --- internal/init_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/init_test.go b/internal/init_test.go index c55aee06f77..af8e7bd7a9b 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -7,6 +7,8 @@ import ( ) func TestBundleInitErrorOnUnknownFields(t *testing.T) { + t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) + tmpDir := t.TempDir() _, _, err := RequireErrorRun(t, "bundle", "init", "./testdata/init/field-does-not-exist", "--output-dir", tmpDir) assert.EqualError(t, err, `failed to compute file content for bar.tmpl. template: :2:2: executing "" at <.does_not_exist>: map has no entry for key "does_not_exist"`) From 34f35f6bc72b6f6eb4b55be036b47dbd8e388ab1 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 13 Sep 2023 16:21:16 +0200 Subject: [PATCH 05/10] add regex capturing for variable name --- libs/template/renderer.go | 11 +++++++++++ libs/template/renderer_test.go | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index adfee71979e..6ba3bfe05fd 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -8,6 +8,7 @@ import ( "os" "path" "path/filepath" + "regexp" "slices" "sort" "strings" @@ -118,6 +119,16 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { result := strings.Builder{} err = tmpl.Execute(&result, r.config) if err != nil { + // Parse and return a more readable error for missing values from input + // config that the template uses. + if strings.Contains(err.Error(), "map has no entry for key") { + captureRegex := regexp.MustCompile(`map has no entry for key "(.*)"`) + matches := captureRegex.FindStringSubmatch(err.Error()) + if len(matches) != 2 { + return "", err + } + return "", fmt.Errorf("unknown input parameter %q used. All inputs for the template should be defined in %s", matches[1], schemaFileName) + } return "", err } return result.String(), nil diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index f6943abfd11..d008438579b 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -198,7 +198,7 @@ func TestRendererExecuteTemplateWithUnknownProperty(t *testing.T) { } _, err := r.executeTemplate(templateText) - assert.ErrorContains(t, err, "map has no entry for key \"does_not_exist\"") + assert.ErrorContains(t, err, `unknown input parameter \"does_not_exist\" used. All inputs for the template should be defined in databricks_template_schema.json`) } func TestRendererIsSkipped(t *testing.T) { From cf2fde86ab00b9e1e2c9f33b9e012c54da00feb1 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 13 Sep 2023 16:27:16 +0200 Subject: [PATCH 06/10] better error message --- internal/init_test.go | 2 +- libs/template/renderer.go | 2 +- libs/template/renderer_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/init_test.go b/internal/init_test.go index af8e7bd7a9b..ec38607d47c 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -11,5 +11,5 @@ func TestBundleInitErrorOnUnknownFields(t *testing.T) { tmpDir := t.TempDir() _, _, err := RequireErrorRun(t, "bundle", "init", "./testdata/init/field-does-not-exist", "--output-dir", tmpDir) - assert.EqualError(t, err, `failed to compute file content for bar.tmpl. template: :2:2: executing "" at <.does_not_exist>: map has no entry for key "does_not_exist"`) + assert.EqualError(t, err, "failed to compute file content for bar.tmpl. variable \"does_not_exist\" not defined") } diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 6ba3bfe05fd..ad20e0d0841 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -127,7 +127,7 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { if len(matches) != 2 { return "", err } - return "", fmt.Errorf("unknown input parameter %q used. All inputs for the template should be defined in %s", matches[1], schemaFileName) + return "", fmt.Errorf("variable %q not defined", matches[1]) } return "", err } diff --git a/libs/template/renderer_test.go b/libs/template/renderer_test.go index d008438579b..8f8a8291d65 100644 --- a/libs/template/renderer_test.go +++ b/libs/template/renderer_test.go @@ -198,7 +198,7 @@ func TestRendererExecuteTemplateWithUnknownProperty(t *testing.T) { } _, err := r.executeTemplate(templateText) - assert.ErrorContains(t, err, `unknown input parameter \"does_not_exist\" used. All inputs for the template should be defined in databricks_template_schema.json`) + assert.ErrorContains(t, err, "variable \"does_not_exist\" not defined") } func TestRendererIsSkipped(t *testing.T) { From 6ee6e428c27c70448e6286828dd30d9a082f9117 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 13 Sep 2023 16:29:15 +0200 Subject: [PATCH 07/10] - --- libs/template/renderer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index ad20e0d0841..d6517cbc01a 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -119,8 +119,8 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { result := strings.Builder{} err = tmpl.Execute(&result, r.config) if err != nil { - // Parse and return a more readable error for missing values from input - // config that the template uses. + // Parse and return a more readable error for missing values that are used + // by the template definition but are not provided in the passed config. if strings.Contains(err.Error(), "map has no entry for key") { captureRegex := regexp.MustCompile(`map has no entry for key "(.*)"`) matches := captureRegex.FindStringSubmatch(err.Error()) From fcea7bef14d14a5a6019fa03ff5b66a7553ddc44 Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 13 Sep 2023 19:17:35 +0200 Subject: [PATCH 08/10] address comments --- libs/template/renderer.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index d6517cbc01a..3697d4069b8 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -99,15 +99,15 @@ func newRenderer(ctx context.Context, config map[string]any, helpers template.Fu func (r *renderer) executeTemplate(templateDefinition string) (string, error) { // Create copy of base template so as to not overwrite it tmpl, err := r.baseTemplate.Clone() + if err != nil { + return "", err + } // The template execution will error instead of printing on unknown // map keys if the "missingkey=error" option is set. // We do this here instead of doing this once for r.baseTemplate because // the Template.Clone() method does not clone options. tmpl = tmpl.Option("missingkey=error") - if err != nil { - return "", err - } // Parse the template text tmpl, err = tmpl.Parse(templateDefinition) @@ -121,9 +121,10 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { if err != nil { // Parse and return a more readable error for missing values that are used // by the template definition but are not provided in the passed config. - if strings.Contains(err.Error(), "map has no entry for key") { + target := &template.ExecError{} + if errors.As(err, target) { captureRegex := regexp.MustCompile(`map has no entry for key "(.*)"`) - matches := captureRegex.FindStringSubmatch(err.Error()) + matches := captureRegex.FindStringSubmatch(target.Err.Error()) if len(matches) != 2 { return "", err } From 9a97509141746ad7f5bbf5872b2b3f8b8e9129fd Mon Sep 17 00:00:00 2001 From: monalisa Date: Wed, 13 Sep 2023 19:25:12 +0200 Subject: [PATCH 09/10] add acc prefix --- internal/init_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/init_test.go b/internal/init_test.go index ec38607d47c..a2eda983c2a 100644 --- a/internal/init_test.go +++ b/internal/init_test.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestBundleInitErrorOnUnknownFields(t *testing.T) { +func TestAccBundleInitErrorOnUnknownFields(t *testing.T) { t.Log(GetEnvOrSkipTest(t, "CLOUD_ENV")) tmpDir := t.TempDir() From d33d1fa73394e5830b39dd4b542f81dd6ea8a4b7 Mon Sep 17 00:00:00 2001 From: monalisa Date: Thu, 14 Sep 2023 13:50:27 +0200 Subject: [PATCH 10/10] address comments --- libs/template/renderer.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/template/renderer.go b/libs/template/renderer.go index 3697d4069b8..09ccc3f5ea3 100644 --- a/libs/template/renderer.go +++ b/libs/template/renderer.go @@ -128,7 +128,10 @@ func (r *renderer) executeTemplate(templateDefinition string) (string, error) { if len(matches) != 2 { return "", err } - return "", fmt.Errorf("variable %q not defined", matches[1]) + return "", template.ExecError{ + Name: target.Name, + Err: fmt.Errorf("variable %q not defined", matches[1]), + } } return "", err }