From 52506694b25075b9ad37c79918cf9bd95fa57026 Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 28 Oct 2025 12:05:18 +0000 Subject: [PATCH 1/2] fix incorrect config key name Fix bug with config validation incorrectly generating error messages omitting the field name segment resulting in the source of configuration problems not being obvious --- changes/1727.bugfix | 1 + utils/config/error.go | 9 +++++++- utils/config/service_configuration_test.go | 25 ++++++++++++++++++++-- 3 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 changes/1727.bugfix diff --git a/changes/1727.bugfix b/changes/1727.bugfix new file mode 100644 index 0000000000..3226dae148 --- /dev/null +++ b/changes/1727.bugfix @@ -0,0 +1 @@ +Fix bug with config validation incorrectly generating error messages omitting the field name segment \ No newline at end of file diff --git a/utils/config/error.go b/utils/config/error.go index bcd6a5d038..96c9acd2ad 100644 --- a/utils/config/error.go +++ b/utils/config/error.go @@ -56,6 +56,7 @@ type validationError struct { mapStructureTree []string mapStructurePrefix *string reason string + fieldName string } func (v *validationError) GetTree() []string { @@ -80,9 +81,11 @@ func (v *validationError) RecordField(fieldName string, mapStructureFieldName *s treeMap = append(treeMap, strings.ToUpper(strings.TrimSpace(*mapStructureFieldName))) treeMap = append(treeMap, v.mapStructureTree...) v.mapStructureTree = treeMap + } else { + v.fieldName = fieldName } - v.mapStructurePrefix = mapStructurePrefix + v.mapStructurePrefix = mapStructurePrefix } func (v *validationError) RecordPrefix(mapStructurePrefix string) { @@ -114,6 +117,10 @@ func (v *validationError) GetMapStructurePath() string { if v.mapStructurePrefix != nil { mapstructureStr = fmt.Sprintf("%v_%v", strings.ToUpper(strings.TrimSpace(*v.mapStructurePrefix)), mapstructureStr) } + + if v.fieldName != "" { + mapstructureStr = fmt.Sprintf("%v_%v", mapstructureStr, strings.ToUpper(strings.TrimSpace(v.fieldName))) + } } return mapstructureStr } diff --git a/utils/config/service_configuration_test.go b/utils/config/service_configuration_test.go index 5df642d2d8..1df950cb6d 100644 --- a/utils/config/service_configuration_test.go +++ b/utils/config/service_configuration_test.go @@ -87,7 +87,15 @@ func DefaultDeepConfiguration() *DeepConfig { } func (cfg *DeepConfig) Validate() error { - return nil + // Validate Embedded Structs + err := ValidateEmbedded(cfg) + if err != nil { + return err + } + + return validation.ValidateStruct(cfg, + validation.Field(&cfg.TestConfigDeep, validation.Required), + ) } func (cfg *ConfigurationTest) Validate() error { @@ -121,8 +129,18 @@ func TestErrorFormatting(t *testing.T) { cfg := DefaultConfiguration() err := cfg.Validate() require.Error(t, err) + errortest.AssertError(t, err, commonerrors.ErrInvalid) - assert.Contains(t, err.Error(), "invalid: structure failed validation: (TestConfig->db) [DUMMYCONFIG] cannot be blank") + assert.Contains(t, err.Error(), "invalid: structure failed validation: (TestConfig->db) [DUMMYCONFIG_DB] cannot be blank") +} + +func TestDeepErrorFormatting(t *testing.T) { + cfg := DefaultDeepConfiguration() + err := cfg.Validate() + require.Error(t, err) + + errortest.AssertError(t, err, commonerrors.ErrInvalid) + assert.Contains(t, err.Error(), "invalid: structure failed validation: (TestConfigDeep->TestConfig->db) [DEEP_CONFIG_DUMMYCONFIG_DB] cannot be blank") } func TestServiceConfigurationLoad(t *testing.T) { @@ -133,6 +151,9 @@ func TestServiceConfigurationLoad(t *testing.T) { err := Load("test", configTest, defaults) // Some required values are missing. require.Error(t, err) + + assert.ErrorContains(t, err, "(TestConfig->db) [TEST_DUMMYCONFIG_DB] cannot be blank") + errortest.RequireError(t, err, commonerrors.ErrInvalid) errortest.RequireError(t, configTest.Validate(), commonerrors.ErrInvalid) From cda15c566c734e2c8c6994e83423f84d3be8a0bf Mon Sep 17 00:00:00 2001 From: Paul Hawxby Date: Tue, 28 Oct 2025 16:13:58 +0000 Subject: [PATCH 2/2] adds a test for defined mapstructure of field --- utils/config/service_configuration_test.go | 23 ++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/utils/config/service_configuration_test.go b/utils/config/service_configuration_test.go index 1df950cb6d..2c2da66a62 100644 --- a/utils/config/service_configuration_test.go +++ b/utils/config/service_configuration_test.go @@ -135,12 +135,31 @@ func TestErrorFormatting(t *testing.T) { } func TestDeepErrorFormatting(t *testing.T) { - cfg := DefaultDeepConfiguration() - err := cfg.Validate() + defaults := DefaultDeepConfiguration() + err := defaults.Validate() require.Error(t, err) errortest.AssertError(t, err, commonerrors.ErrInvalid) assert.Contains(t, err.Error(), "invalid: structure failed validation: (TestConfigDeep->TestConfig->db) [DEEP_CONFIG_DUMMYCONFIG_DB] cannot be blank") + + err = os.Setenv("TEST_DEEP_CONFIG_DUMMYCONFIG_DB", "a test db") + require.NoError(t, err) + err = os.Setenv("TEST_DEEP_CONFIG_DUMMYCONFIG_DUMMY_HOST", "a test host") + require.NoError(t, err) + err = os.Setenv("TEST_DEEP_CONFIG_DUMMYCONFIG_PASSWORD", "a test password") + require.NoError(t, err) + err = os.Setenv("TEST_DEEP_CONFIG_DUMMYCONFIG_USER", "a test user") + require.NoError(t, err) + err = os.Setenv("TEST_DEEP_CONFIG_DUMMY_CONFIG_DB", "a test user") + require.NoError(t, err) + + t.Run("defined mapstructure", func(t *testing.T) { + configTest2 := &DeepConfig{} + err = LoadFromSystem("test", configTest2, defaults) + + errortest.AssertError(t, err, commonerrors.ErrInvalid) + assert.Contains(t, err.Error(), "invalid: structure failed validation: (TestConfigDeep->TestConfig2->dummy_host) [TEST_DEEP_CONFIG_DUMMY_CONFIG_DUMMY_HOST] cannot be blank") + }) } func TestServiceConfigurationLoad(t *testing.T) {