From b50380471ed3661b25d7c4fe4f37ca433b8d45fe Mon Sep 17 00:00:00 2001 From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com> Date: Tue, 26 Mar 2024 18:32:09 +0530 Subject: [PATCH] Allow unknown properties in the config file for template initialization (#1315) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Changes Before we would error if a property was defined in the config file, that was not defined in the schema. ## Tests Unit tests. Also manually that the e2e flow works file. Before: ``` shreyas.goenka@THW32HFW6T playground % cli bundle init default-python --config-file config.json Welcome to the default Python template for Databricks Asset Bundles! Error: failed to load config from file config.json: property include_pytho is not defined in the schema ``` After: ``` shreyas.goenka@THW32HFW6T playground % cli bundle init default-python --config-file config.json Welcome to the default Python template for Databricks Asset Bundles! Workspace to use (auto-detected, edit in 'test/databricks.yml'): https://dbc-a39a1eb1-ef95.cloud.databricks.com ✨ Your new project has been created in the 'test' directory! Please refer to the README.md file for "getting started" instructions. See also the documentation at https://docs.databricks.com/dev-tools/bundles/index.html. ``` --- libs/template/config.go | 13 ++++++++++++- libs/template/config_test.go | 11 +++++++++++ .../config.json | 3 ++- 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/libs/template/config.go b/libs/template/config.go index 970e74ca..5470aefe 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -70,8 +70,14 @@ func validateSchema(schema *jsonschema.Schema) error { // Reads json file at path and assigns values from the file func (c *config) assignValuesFromFile(path string) error { - // Load the config file. + // It's valid to set additional properties in the config file that are not + // defined in the schema. They will be filtered below. Thus for the duration of + // the LoadInstance call, we disable the additional properties check, + // to allow those properties to be loaded. + c.schema.AdditionalProperties = true configFromFile, err := c.schema.LoadInstance(path) + c.schema.AdditionalProperties = false + if err != nil { return fmt.Errorf("failed to load config from file %s: %w", path, err) } @@ -79,6 +85,11 @@ func (c *config) assignValuesFromFile(path string) error { // Write configs from the file to the input map, not overwriting any existing // configurations. for name, val := range configFromFile { + // If a property is not defined in the schema, skip it. + if _, ok := c.schema.Properties[name]; !ok { + continue + } + // If a value is already assigned, keep the original value. if _, ok := c.values[name]; ok { continue } diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 847c2615..1af2e5f5 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -52,6 +52,17 @@ func TestTemplateConfigAssignValuesFromFileDoesNotOverwriteExistingConfigs(t *te assert.Equal(t, "this-is-not-overwritten", c.values["string_val"]) } +func TestTemplateConfigAssignValuesFromFileFiltersPropertiesNotInTheSchema(t *testing.T) { + c := testConfig(t) + + err := c.assignValuesFromFile("./testdata/config-assign-from-file-unknown-property/config.json") + assert.NoError(t, err) + + // assert only the known property is loaded + assert.Len(t, c.values, 1) + assert.Equal(t, "i am a known property", c.values["string_val"]) +} + func TestTemplateConfigAssignDefaultValues(t *testing.T) { c := testConfig(t) diff --git a/libs/template/testdata/config-assign-from-file-unknown-property/config.json b/libs/template/testdata/config-assign-from-file-unknown-property/config.json index 518eaa6a..69ed020c 100644 --- a/libs/template/testdata/config-assign-from-file-unknown-property/config.json +++ b/libs/template/testdata/config-assign-from-file-unknown-property/config.json @@ -1,3 +1,4 @@ { - "unknown_prop": 123 + "unknown_prop": 123, + "string_val": "i am a known property" }