From 09fb7f6eb5d573aabe7cbbefae244db674c80a06 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 26 Mar 2024 11:50:07 +0100 Subject: [PATCH 1/4] Allow unknown properties in the config file for template initialization --- libs/template/config.go | 14 ++++++++++++++ libs/template/config_test.go | 9 +++++++++ 2 files changed, 23 insertions(+) diff --git a/libs/template/config.go b/libs/template/config.go index 970e74ca95..4f6d49f6ab 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -70,6 +70,15 @@ func validateSchema(schema *jsonschema.Schema) error { // Reads json file at path and assigns values from the file func (c *config) assignValuesFromFile(path string) error { + // It's valid to set additional properties in the config file that are not + // defined in the schema. They are filtered below. For the duration of this + // function we disable the additional properties check, to allow those + // properties to be loaded. + c.schema.AdditionalProperties = true + defer func() { + c.schema.AdditionalProperties = false + }() + // Load the config file. configFromFile, err := c.schema.LoadInstance(path) if err != nil { @@ -79,6 +88,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 it. if _, ok := c.values[name]; ok { continue } diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 847c2615bb..83aabba69b 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -52,6 +52,15 @@ 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.Len(t, c.values, 0) +} + func TestTemplateConfigAssignDefaultValues(t *testing.T) { c := testConfig(t) From 674f6192beaf2dab4b9d2e768012d014aa839dba Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 26 Mar 2024 11:55:45 +0100 Subject: [PATCH 2/4] - --- libs/template/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/template/config.go b/libs/template/config.go index 4f6d49f6ab..0eb9e7b53b 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -92,7 +92,7 @@ func (c *config) assignValuesFromFile(path string) error { if _, ok := c.schema.Properties[name]; !ok { continue } - // If a value is already assigned, keep it. + // If a value is already assigned, keep the original value. if _, ok := c.values[name]; ok { continue } From 1b4cbd3001f88ad64f5060168325fe36c09a46b0 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 26 Mar 2024 13:25:42 +0100 Subject: [PATCH 3/4] - --- libs/template/config.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/libs/template/config.go b/libs/template/config.go index 0eb9e7b53b..5470aefeb6 100644 --- a/libs/template/config.go +++ b/libs/template/config.go @@ -71,16 +71,13 @@ func validateSchema(schema *jsonschema.Schema) error { // Reads json file at path and assigns values from the file func (c *config) assignValuesFromFile(path string) error { // It's valid to set additional properties in the config file that are not - // defined in the schema. They are filtered below. For the duration of this - // function we disable the additional properties check, to allow those - // properties to be loaded. + // 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 - defer func() { - c.schema.AdditionalProperties = false - }() - - // Load the config file. 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) } From 27ed432b0b15dc186c00a3ceb8effb07a61d1421 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Tue, 26 Mar 2024 13:28:50 +0100 Subject: [PATCH 4/4] add assertion for known property --- libs/template/config_test.go | 4 +++- .../config-assign-from-file-unknown-property/config.json | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/template/config_test.go b/libs/template/config_test.go index 83aabba69b..1af2e5f5ae 100644 --- a/libs/template/config_test.go +++ b/libs/template/config_test.go @@ -58,7 +58,9 @@ func TestTemplateConfigAssignValuesFromFileFiltersPropertiesNotInTheSchema(t *te err := c.assignValuesFromFile("./testdata/config-assign-from-file-unknown-property/config.json") assert.NoError(t, err) - assert.Len(t, c.values, 0) + // 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) { 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 518eaa6a26..69ed020cf9 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" }