From 6c927cb832b2eb7f6e9724eb6398244dfa0333eb Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Thu, 25 Sep 2025 19:44:53 +0100 Subject: [PATCH 1/6] Automatically flatten embedded struct fields into parent structs and map them correctly --- .../flat-config-to-embedded-test.json | 6 + utils/config/service_configuration.go | 6 +- utils/config/service_configuration_test.go | 106 ++++++++++++++++++ utils/serialization/maps/map.go | 1 + utils/serialization/maps/map_test.go | 41 +++++++ 5 files changed, 159 insertions(+), 1 deletion(-) create mode 100644 utils/config/fixtures/flat-config-to-embedded-test.json diff --git a/utils/config/fixtures/flat-config-to-embedded-test.json b/utils/config/fixtures/flat-config-to-embedded-test.json new file mode 100644 index 0000000000..12ce86ec88 --- /dev/null +++ b/utils/config/fixtures/flat-config-to-embedded-test.json @@ -0,0 +1,6 @@ +{ + "non_embedded1": "non_embedded 1", + "embedded1": "embedded 1", + "embedded2": "embedded 2", + "embedded_struct_tag": "embedded struct tag" +} \ No newline at end of file diff --git a/utils/config/service_configuration.go b/utils/config/service_configuration.go index f0c5cf0cf7..f29ada19f0 100644 --- a/utils/config/service_configuration.go +++ b/utils/config/service_configuration.go @@ -112,8 +112,12 @@ func LoadFromEnvironmentAndSystem(viperSession *viper.Viper, envVarPrefix string } } + defaultDecodeConfigOptions := func(dc *mapstructure.DecoderConfig) { + dc.Squash = true // this will automatically flatten embedded struct fields into parent structs and map them correctly + } + // Merge together all the sources and unmarshal into struct - err = viperSession.Unmarshal(configurationToSet) + err = viperSession.Unmarshal(configurationToSet, defaultDecodeConfigOptions) if err != nil { err = commonerrors.WrapError(commonerrors.ErrMarshalling, err, "unable to fill configuration structure from the configuration session") return diff --git a/utils/config/service_configuration_test.go b/utils/config/service_configuration_test.go index b0566cb6f5..edda65c674 100644 --- a/utils/config/service_configuration_test.go +++ b/utils/config/service_configuration_test.go @@ -776,3 +776,109 @@ func TestServiceConfigurationLoadFromEnvironment(t *testing.T) { assert.True(t, configTest.TestConfig.Flag) assert.False(t, configTest.TestConfig2.Flag) } + +type TestBaseCfg struct { + Embedded1 string `mapstructure:"embedded1"` + Embedded2 string `mapstructure:"embedded2"` +} + +func (cfg *TestBaseCfg) Validate() error { + return validation.ValidateStruct(cfg, + validation.Field(&cfg.Embedded1, validation.Required), + validation.Field(&cfg.Embedded2, validation.Required), + ) +} + +type TestCfgWithEmbeddedCfg struct { + TestBaseCfg + NonEmbedded1 string `mapstructure:"non_embedded1"` + NonEmbedded2 string `mapstructure:"non_embedded2"` +} + +func (cfg *TestCfgWithEmbeddedCfg) Validate() error { + // Validate Embedded Structs + err := ValidateEmbedded(cfg) + if err != nil { + return err + } + + return validation.ValidateStruct(cfg, + validation.Field(&cfg.NonEmbedded1, validation.Required), + ) +} + +type TestCfgWithEmbeddedCfgWithTag struct { + TestBaseCfg `mapstructure:"embedded_struct_tag"` + NonEmbedded1 string `mapstructure:"non_embedded1"` +} + +func (cfg *TestCfgWithEmbeddedCfgWithTag) Validate() error { + // Validate Embedded Structs + err := ValidateEmbedded(cfg) + if err != nil { + return err + } + + return validation.ValidateStruct(cfg, + validation.Field(&cfg.NonEmbedded1, validation.Required), + ) +} + +// Config values should be correctly mapped onto a struct that embeds another struct +func TestEmbeddedServiceConfigurationLoadFromFile(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbedded := TestCfgWithEmbeddedCfg{} + err := LoadFromEnvironment(session, "", &testEmbedded, + &TestCfgWithEmbeddedCfg{}, filepath.Join(".", "fixtures", "flat-config-to-embedded-test.json")) + require.NoError(t, err) + assert.NotEmpty(t, testEmbedded.NonEmbedded1) + assert.Equal(t, "non_embedded 1", testEmbedded.NonEmbedded1) + assert.Empty(t, testEmbedded.NonEmbedded2) + assert.NotEmpty(t, testEmbedded.Embedded1) + assert.Equal(t, "embedded 1", testEmbedded.Embedded1) + assert.NotEmpty(t, testEmbedded.Embedded2) + assert.Equal(t, "embedded 2", testEmbedded.Embedded2) +} + +// Config values should be correctly mapped onto a struct that embeds another struct tagged with mapstructure +func TestEmbeddedServiceConfigurationWithTagLoadFromFile(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbeddedWithCustomTag := TestCfgWithEmbeddedCfgWithTag{} + err := LoadFromEnvironment(session, "", &testEmbeddedWithCustomTag, + &TestCfgWithEmbeddedCfgWithTag{}, filepath.Join(".", "fixtures", "flat-config-to-embedded-test.json")) + require.NoError(t, err) + assert.NotEmpty(t, testEmbeddedWithCustomTag.NonEmbedded1) + assert.Equal(t, "non_embedded 1", testEmbeddedWithCustomTag.NonEmbedded1) + assert.NotEmpty(t, testEmbeddedWithCustomTag.Embedded1) + assert.Equal(t, "embedded 1", testEmbeddedWithCustomTag.Embedded1) + assert.NotEmpty(t, testEmbeddedWithCustomTag.Embedded2) + assert.Equal(t, "embedded 2", testEmbeddedWithCustomTag.Embedded2) +} + +// Config values should be correctly mapped onto a struct that embeds another struct and maintain any defaults not overwritten +func TestEmbeddedServiceConfigurationLoadFromFileWithDefaults(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbedded := TestCfgWithEmbeddedCfg{} + defaults := &TestCfgWithEmbeddedCfg{ + TestBaseCfg: TestBaseCfg{ + Embedded1: "a", + Embedded2: "b", + }, + NonEmbedded1: "c", + NonEmbedded2: "d", + } + err := LoadFromEnvironment(session, "", &testEmbedded, + defaults, filepath.Join(".", "fixtures", "flat-config-to-embedded-test.json")) + require.NoError(t, err) + assert.NotEmpty(t, testEmbedded.NonEmbedded1) + assert.Equal(t, "non_embedded 1", testEmbedded.NonEmbedded1) + assert.NotEmpty(t, testEmbedded.NonEmbedded2) + assert.Equal(t, "d", testEmbedded.NonEmbedded2) + assert.NotEmpty(t, testEmbedded.Embedded1) + assert.Equal(t, "embedded 1", testEmbedded.Embedded1) + assert.NotEmpty(t, testEmbedded.Embedded2) + assert.Equal(t, "embedded 2", testEmbedded.Embedded2) +} diff --git a/utils/serialization/maps/map.go b/utils/serialization/maps/map.go index 095656a44e..72b46e6317 100644 --- a/utils/serialization/maps/map.go +++ b/utils/serialization/maps/map.go @@ -154,6 +154,7 @@ func mapstructureDecoder(input, result any) error { DecodeHook: mapstructure.ComposeDecodeHookFunc( timeHookFunc(), mapstructure.StringToTimeDurationHookFunc(), mapstructure.StringToURLHookFunc(), mapstructure.StringToIPHookFunc()), Result: result, + Squash: true, }) if err != nil { return err diff --git a/utils/serialization/maps/map_test.go b/utils/serialization/maps/map_test.go index 9389dee138..581612bfe8 100644 --- a/utils/serialization/maps/map_test.go +++ b/utils/serialization/maps/map_test.go @@ -100,12 +100,28 @@ type TestStruct2WithTime struct { Time time.Time `mapstructure:"some_time"` Duration time.Duration `mapstructure:"some_duration"` } + type TestStruct3WithTime struct { Time time.Time `mapstructure:"some_time"` Duration time.Duration `mapstructure:"some_duration"` Struct TestStruct2WithTime } +type TestStruct4 struct { + Field1 string `mapstructure:"field_1"` + Field2 string `mapstructure:"field_2"` +} + +type TestStruct5WithEmbeddedStruct struct { + TestStruct4 + Field3 string `mapstructure:"field_3"` +} + +type TestStruct6WithEmbeddedStruct struct { + TestStruct5WithEmbeddedStruct + Field4 string `mapstructure:"field_4"` +} + func TestToMap(t *testing.T) { t.Run("generic", func(t *testing.T) { testStruct := TestStruct1{} @@ -160,5 +176,30 @@ func TestToMap(t *testing.T) { errortest.AssertError(t, FromMapToPointer[any](testMap, nil), commonerrors.ErrInvalid, commonerrors.ErrUndefined) errortest.AssertError(t, FromMapToPointer[*TestStruct3WithTime](testMap, nil), commonerrors.ErrInvalid, commonerrors.ErrUndefined) }) + t.Run("embedded struct mapping", func(t *testing.T) { + testStruct := TestStruct6WithEmbeddedStruct{} + require.NoError(t, faker.FakeData(&testStruct)) + m := make(map[string]string) + m["field_1"] = faker.Word() + m["field_2"] = faker.Word() + m["field_3"] = faker.Word() + m["field_4"] = faker.Word() + + // Should correctly set embedded struct fields from map m, even on nested embedded structs + err := FromMap[TestStruct6WithEmbeddedStruct](m, &testStruct) + require.NoError(t, err) + assert.Equal(t, testStruct.Field1, m["field_1"]) + assert.Equal(t, testStruct.Field2, m["field_2"]) + assert.Equal(t, testStruct.Field3, m["field_3"]) + assert.Equal(t, testStruct.Field4, m["field_4"]) + + structMap, err := ToMap[TestStruct6WithEmbeddedStruct](&testStruct) + require.NoError(t, err) + assert.Equal(t, m, structMap) + + newStruct := TestStruct6WithEmbeddedStruct{} + require.NoError(t, FromMap[TestStruct6WithEmbeddedStruct](structMap, &newStruct)) + assert.Equal(t, testStruct, newStruct) + }) } From 25671d6632154f34c42c0ececd20eb00ae638492 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Thu, 25 Sep 2025 20:46:27 +0100 Subject: [PATCH 2/6] Add news file --- changes/20250925204435.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/20250925204435.feature diff --git a/changes/20250925204435.feature b/changes/20250925204435.feature new file mode 100644 index 0000000000..2321b73e68 --- /dev/null +++ b/changes/20250925204435.feature @@ -0,0 +1 @@ +:sparkles: `[config]` Automatically flatten embedded struct fields From 089e676cb73e835c3ae7ce9b9d4cd4ae3a8c71e5 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Fri, 26 Sep 2025 08:59:45 +0100 Subject: [PATCH 3/6] Update news file --- changes/20250925204435.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changes/20250925204435.feature b/changes/20250925204435.feature index 2321b73e68..1a9733c880 100644 --- a/changes/20250925204435.feature +++ b/changes/20250925204435.feature @@ -1 +1 @@ -:sparkles: `[config]` Automatically flatten embedded struct fields +:sparkles: `[config]` Automatically flatten embedded struct fields on service configurations From 84708ee3b1fc92fbfc972748f3c1635c7088cb83 Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Fri, 26 Sep 2025 13:24:49 +0100 Subject: [PATCH 4/6] Use `mapstructure:",squash"` field tag and revert boolean Squash change --- changes/20250925204435.feature | 2 +- utils/config/fixtures/env-test.env | 15 ++ utils/config/fixtures/flat-config-test.json | 5 + .../flat-config-to-embedded-test.json | 6 - utils/config/fixtures/nested-config-test.json | 11 ++ utils/config/service_configuration.go | 6 +- utils/config/service_configuration_test.go | 154 ++++++++++++++++-- utils/serialization/maps/map.go | 1 - utils/serialization/maps/map_test.go | 44 ++++- 9 files changed, 209 insertions(+), 35 deletions(-) create mode 100644 utils/config/fixtures/env-test.env create mode 100644 utils/config/fixtures/flat-config-test.json delete mode 100644 utils/config/fixtures/flat-config-to-embedded-test.json create mode 100644 utils/config/fixtures/nested-config-test.json diff --git a/changes/20250925204435.feature b/changes/20250925204435.feature index 1a9733c880..5a106dd3c5 100644 --- a/changes/20250925204435.feature +++ b/changes/20250925204435.feature @@ -1 +1 @@ -:sparkles: `[config]` Automatically flatten embedded struct fields on service configurations +:gear: `[config]` Add tests for service configurations with embedded structs tagged with `mapstructure:",squash"` diff --git a/utils/config/fixtures/env-test.env b/utils/config/fixtures/env-test.env new file mode 100644 index 0000000000..76b5c82ab9 --- /dev/null +++ b/utils/config/fixtures/env-test.env @@ -0,0 +1,15 @@ +# These env vars will map to a struct the embeds another struct with no mapstructure tag set (it will expect it's struct name as part of the env var) +WITH_NO_TAG_TESTBASECFG_EMBEDDED1="embedded 1" +WITH_NO_TAG_TESTBASECFG_EMBEDDED2="embedded 2" +WITH_NO_TAG_NON_EMBEDDED1="non-embedded 1" +WITH_NO_TAG_NON_EMBEDDED2="non-embedded 2" +# These env vars will map to a struct the embeds another struct with a mapstructure tag set +WITH_TAG_EMBEDDED_STRUCT_EMBEDDED1="embedded 1" +WITH_TAG_EMBEDDED_STRUCT_EMBEDDED2="embedded 2" +WITH_TAG_NON_EMBEDDED1="non-embedded 1" +WITH_TAG_NON_EMBEDDED2="non-embedded 2" +# These env vars will map to a struct the embeds another struct with the ",squash" mapstructure tag set +WITH_SQUASH_TAG_EMBEDDED1="embedded 1" +WITH_SQUASH_TAG_EMBEDDED2="embedded 2" +WITH_SQUASH_TAG_NON_EMBEDDED1="non-embedded 1" +WITH_SQUASH_TAG_NON_EMBEDDED2="non-embedded 2" \ No newline at end of file diff --git a/utils/config/fixtures/flat-config-test.json b/utils/config/fixtures/flat-config-test.json new file mode 100644 index 0000000000..dd0b9f82d5 --- /dev/null +++ b/utils/config/fixtures/flat-config-test.json @@ -0,0 +1,5 @@ +{ + "embedded1": "embedded 1", + "embedded2": "embedded 2", + "non_embedded1": "non-embedded 1" +} \ No newline at end of file diff --git a/utils/config/fixtures/flat-config-to-embedded-test.json b/utils/config/fixtures/flat-config-to-embedded-test.json deleted file mode 100644 index 12ce86ec88..0000000000 --- a/utils/config/fixtures/flat-config-to-embedded-test.json +++ /dev/null @@ -1,6 +0,0 @@ -{ - "non_embedded1": "non_embedded 1", - "embedded1": "embedded 1", - "embedded2": "embedded 2", - "embedded_struct_tag": "embedded struct tag" -} \ No newline at end of file diff --git a/utils/config/fixtures/nested-config-test.json b/utils/config/fixtures/nested-config-test.json new file mode 100644 index 0000000000..d46efa0ec5 --- /dev/null +++ b/utils/config/fixtures/nested-config-test.json @@ -0,0 +1,11 @@ +{ + "testbasecfg": { + "embedded1": "embedded 1", + "embedded2": "embedded 2" + }, + "embedded_struct": { + "embedded1": "embedded 1", + "embedded2": "embedded 2" + }, + "non_embedded1": "non-embedded 1" +} \ No newline at end of file diff --git a/utils/config/service_configuration.go b/utils/config/service_configuration.go index f29ada19f0..f0c5cf0cf7 100644 --- a/utils/config/service_configuration.go +++ b/utils/config/service_configuration.go @@ -112,12 +112,8 @@ func LoadFromEnvironmentAndSystem(viperSession *viper.Viper, envVarPrefix string } } - defaultDecodeConfigOptions := func(dc *mapstructure.DecoderConfig) { - dc.Squash = true // this will automatically flatten embedded struct fields into parent structs and map them correctly - } - // Merge together all the sources and unmarshal into struct - err = viperSession.Unmarshal(configurationToSet, defaultDecodeConfigOptions) + err = viperSession.Unmarshal(configurationToSet) if err != nil { err = commonerrors.WrapError(commonerrors.ErrMarshalling, err, "unable to fill configuration structure from the configuration session") return diff --git a/utils/config/service_configuration_test.go b/utils/config/service_configuration_test.go index edda65c674..e9f13a9f43 100644 --- a/utils/config/service_configuration_test.go +++ b/utils/config/service_configuration_test.go @@ -8,6 +8,7 @@ import ( "context" "errors" "fmt" + "io/fs" "math/rand" "os" "path/filepath" @@ -16,6 +17,7 @@ import ( "github.com/go-faker/faker/v4" validation "github.com/go-ozzo/ozzo-validation/v4" + "github.com/joho/godotenv" "github.com/spf13/pflag" "github.com/spf13/viper" "github.com/stretchr/testify/assert" @@ -808,7 +810,7 @@ func (cfg *TestCfgWithEmbeddedCfg) Validate() error { } type TestCfgWithEmbeddedCfgWithTag struct { - TestBaseCfg `mapstructure:"embedded_struct_tag"` + TestBaseCfg `mapstructure:"embedded_struct"` NonEmbedded1 string `mapstructure:"non_embedded1"` } @@ -824,16 +826,33 @@ func (cfg *TestCfgWithEmbeddedCfgWithTag) Validate() error { ) } -// Config values should be correctly mapped onto a struct that embeds another struct -func TestEmbeddedServiceConfigurationLoadFromFile(t *testing.T) { +type TestCfgWithEmbeddedCfgWithSquashTag struct { + TestBaseCfg `mapstructure:",squash"` + NonEmbedded1 string `mapstructure:"non_embedded1"` +} + +func (cfg *TestCfgWithEmbeddedCfgWithSquashTag) Validate() error { + // Validate Embedded Structs + err := ValidateEmbedded(cfg) + if err != nil { + return err + } + + return validation.ValidateStruct(cfg, + validation.Field(&cfg.NonEmbedded1, validation.Required), + ) +} + +// Config values loaded from file should be correctly mapped onto a struct that embeds another struct with no mapstructure tag set +func TestEmbeddedServiceConfigurationWithNoTagLoadFromFile(t *testing.T) { os.Clearenv() session := viper.New() testEmbedded := TestCfgWithEmbeddedCfg{} err := LoadFromEnvironment(session, "", &testEmbedded, - &TestCfgWithEmbeddedCfg{}, filepath.Join(".", "fixtures", "flat-config-to-embedded-test.json")) + &TestCfgWithEmbeddedCfg{}, filepath.Join(".", "fixtures", "nested-config-test.json")) require.NoError(t, err) assert.NotEmpty(t, testEmbedded.NonEmbedded1) - assert.Equal(t, "non_embedded 1", testEmbedded.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbedded.NonEmbedded1) assert.Empty(t, testEmbedded.NonEmbedded2) assert.NotEmpty(t, testEmbedded.Embedded1) assert.Equal(t, "embedded 1", testEmbedded.Embedded1) @@ -841,23 +860,54 @@ func TestEmbeddedServiceConfigurationLoadFromFile(t *testing.T) { assert.Equal(t, "embedded 2", testEmbedded.Embedded2) } -// Config values should be correctly mapped onto a struct that embeds another struct tagged with mapstructure +// Config values loaded from file should be correctly mapped onto a struct that embeds another struct with a mapstructure tag set func TestEmbeddedServiceConfigurationWithTagLoadFromFile(t *testing.T) { os.Clearenv() session := viper.New() - testEmbeddedWithCustomTag := TestCfgWithEmbeddedCfgWithTag{} - err := LoadFromEnvironment(session, "", &testEmbeddedWithCustomTag, - &TestCfgWithEmbeddedCfgWithTag{}, filepath.Join(".", "fixtures", "flat-config-to-embedded-test.json")) + testEmbeddedWithTag := TestCfgWithEmbeddedCfgWithTag{} + err := LoadFromEnvironment(session, "", &testEmbeddedWithTag, + &TestCfgWithEmbeddedCfgWithTag{}, filepath.Join(".", "fixtures", "nested-config-test.json")) require.NoError(t, err) - assert.NotEmpty(t, testEmbeddedWithCustomTag.NonEmbedded1) - assert.Equal(t, "non_embedded 1", testEmbeddedWithCustomTag.NonEmbedded1) - assert.NotEmpty(t, testEmbeddedWithCustomTag.Embedded1) - assert.Equal(t, "embedded 1", testEmbeddedWithCustomTag.Embedded1) - assert.NotEmpty(t, testEmbeddedWithCustomTag.Embedded2) - assert.Equal(t, "embedded 2", testEmbeddedWithCustomTag.Embedded2) + assert.NotEmpty(t, testEmbeddedWithTag.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbeddedWithTag.NonEmbedded1) + assert.NotEmpty(t, testEmbeddedWithTag.Embedded1) + assert.Equal(t, "embedded 1", testEmbeddedWithTag.Embedded1) + assert.NotEmpty(t, testEmbeddedWithTag.Embedded2) + assert.Equal(t, "embedded 2", testEmbeddedWithTag.Embedded2) } -// Config values should be correctly mapped onto a struct that embeds another struct and maintain any defaults not overwritten +// Flat config values loaded from file should be correctly mapped onto a struct that embeds another struct with the ",squash" mapstructure tag set +func TestEmbeddedServiceConfigurationWithSquashTagLoadFromFile(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbeddedWithSquashTag := TestCfgWithEmbeddedCfgWithSquashTag{} + err := LoadFromEnvironment(session, "", &testEmbeddedWithSquashTag, + &TestCfgWithEmbeddedCfgWithSquashTag{}, filepath.Join(".", "fixtures", "flat-config-test.json")) + require.NoError(t, err) + assert.NotEmpty(t, testEmbeddedWithSquashTag.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbeddedWithSquashTag.NonEmbedded1) + assert.NotEmpty(t, testEmbeddedWithSquashTag.Embedded1) + assert.Equal(t, "embedded 1", testEmbeddedWithSquashTag.Embedded1) + assert.NotEmpty(t, testEmbeddedWithSquashTag.Embedded2) + assert.Equal(t, "embedded 2", testEmbeddedWithSquashTag.Embedded2) +} + +// Nested config values loaded from file should not be correctly mapped onto a struct that embeds another struct with the ",squash" mapstructure tag set +func TestEmbeddedServiceConfigurationWithSquashTagLoadFromNestedFile(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbeddedWithSquashTag := TestCfgWithEmbeddedCfgWithSquashTag{} + err := LoadFromEnvironment(session, "", &testEmbeddedWithSquashTag, + &TestCfgWithEmbeddedCfgWithSquashTag{}, filepath.Join(".", "fixtures", "nested-config-test.json")) + require.Error(t, err) + assert.NotEmpty(t, testEmbeddedWithSquashTag.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbeddedWithSquashTag.NonEmbedded1) + assert.Empty(t, testEmbeddedWithSquashTag.Embedded1) + assert.NotEqual(t, "embedded 1", testEmbeddedWithSquashTag.Embedded1) + assert.NotEqual(t, "embedded 2", testEmbeddedWithSquashTag.Embedded2) +} + +// Config values loaded from file should be correctly mapped onto a struct that embeds another struct and maintains any defaults not overwritten func TestEmbeddedServiceConfigurationLoadFromFileWithDefaults(t *testing.T) { os.Clearenv() session := viper.New() @@ -871,10 +921,10 @@ func TestEmbeddedServiceConfigurationLoadFromFileWithDefaults(t *testing.T) { NonEmbedded2: "d", } err := LoadFromEnvironment(session, "", &testEmbedded, - defaults, filepath.Join(".", "fixtures", "flat-config-to-embedded-test.json")) + defaults, filepath.Join(".", "fixtures", "nested-config-test.json")) require.NoError(t, err) assert.NotEmpty(t, testEmbedded.NonEmbedded1) - assert.Equal(t, "non_embedded 1", testEmbedded.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbedded.NonEmbedded1) assert.NotEmpty(t, testEmbedded.NonEmbedded2) assert.Equal(t, "d", testEmbedded.NonEmbedded2) assert.NotEmpty(t, testEmbedded.Embedded1) @@ -882,3 +932,71 @@ func TestEmbeddedServiceConfigurationLoadFromFileWithDefaults(t *testing.T) { assert.NotEmpty(t, testEmbedded.Embedded2) assert.Equal(t, "embedded 2", testEmbedded.Embedded2) } + +// Config values loaded from env vars should be correctly mapped onto a struct that embeds another struct with no mapstructure tag set +func TestEmbeddedServiceConfigurationWithNoTagLoadFromEnvironment(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbedded := TestCfgWithEmbeddedCfg{} + err := loadEnvIntoEnvironment(t, filepath.Join(".", "fixtures", "env-test.env")) + require.NoError(t, err) + + err = LoadFromEnvironment(session, "WITH_NO_TAG", &testEmbedded, + &TestCfgWithEmbeddedCfg{}, "") + require.NoError(t, err) + assert.NotEmpty(t, testEmbedded.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbedded.NonEmbedded1) + assert.NotEmpty(t, testEmbedded.Embedded1) + assert.Equal(t, "embedded 1", testEmbedded.Embedded1) + assert.NotEmpty(t, testEmbedded.Embedded2) + assert.Equal(t, "embedded 2", testEmbedded.Embedded2) +} + +// Config values loaded from env vars should be correctly mapped onto a struct that embeds another struct with a mapstructure tag set +func TestEmbeddedServiceConfigurationWithTagLoadFromEnvironment(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbeddedWithTag := TestCfgWithEmbeddedCfgWithTag{} + err := loadEnvIntoEnvironment(t, filepath.Join(".", "fixtures", "env-test.env")) + require.NoError(t, err) + + err = LoadFromEnvironment(session, "WITH_TAG", &testEmbeddedWithTag, + &TestCfgWithEmbeddedCfgWithTag{}, "") + require.NoError(t, err) + assert.NotEmpty(t, testEmbeddedWithTag.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbeddedWithTag.NonEmbedded1) + assert.NotEmpty(t, testEmbeddedWithTag.Embedded1) + assert.Equal(t, "embedded 1", testEmbeddedWithTag.Embedded1) + assert.NotEmpty(t, testEmbeddedWithTag.Embedded2) + assert.Equal(t, "embedded 2", testEmbeddedWithTag.Embedded2) +} + +// Flat config values loaded from env vars should be correctly mapped onto a struct that embeds another struct with the ",squash" mapstructure tag set +func TestEmbeddedServiceConfigurationWithSquashTagLoadFromEnvironment(t *testing.T) { + os.Clearenv() + session := viper.New() + testEmbeddedWithSquashTag := TestCfgWithEmbeddedCfgWithSquashTag{} + err := loadEnvIntoEnvironment(t, filepath.Join(".", "fixtures", "env-test.env")) + require.NoError(t, err) + + err = LoadFromEnvironment(session, "WITH_SQUASH_TAG", &testEmbeddedWithSquashTag, + &TestCfgWithEmbeddedCfgWithSquashTag{}, "") + require.NoError(t, err) + assert.NotEmpty(t, testEmbeddedWithSquashTag.NonEmbedded1) + assert.Equal(t, "non-embedded 1", testEmbeddedWithSquashTag.NonEmbedded1) + assert.NotEmpty(t, testEmbeddedWithSquashTag.Embedded1) + assert.Equal(t, "embedded 1", testEmbeddedWithSquashTag.Embedded1) + assert.NotEmpty(t, testEmbeddedWithSquashTag.Embedded2) + assert.Equal(t, "embedded 2", testEmbeddedWithSquashTag.Embedded2) +} + +func loadEnvIntoEnvironment(t *testing.T, envPath string) (err error) { + t.Helper() + _, err = fs.Stat(os.DirFS("."), envPath) + require.NoError(t, err) + + err = godotenv.Load(envPath) + require.NoError(t, err) + + return +} diff --git a/utils/serialization/maps/map.go b/utils/serialization/maps/map.go index 72b46e6317..095656a44e 100644 --- a/utils/serialization/maps/map.go +++ b/utils/serialization/maps/map.go @@ -154,7 +154,6 @@ func mapstructureDecoder(input, result any) error { DecodeHook: mapstructure.ComposeDecodeHookFunc( timeHookFunc(), mapstructure.StringToTimeDurationHookFunc(), mapstructure.StringToURLHookFunc(), mapstructure.StringToIPHookFunc()), Result: result, - Squash: true, }) if err != nil { return err diff --git a/utils/serialization/maps/map_test.go b/utils/serialization/maps/map_test.go index 581612bfe8..a63c83ab2c 100644 --- a/utils/serialization/maps/map_test.go +++ b/utils/serialization/maps/map_test.go @@ -117,11 +117,21 @@ type TestStruct5WithEmbeddedStruct struct { Field3 string `mapstructure:"field_3"` } +type TestStruct5WithEmbeddedStructAndSquashTag struct { + TestStruct4 `mapstructure:",squash"` + Field3 string `mapstructure:"field_3"` +} + type TestStruct6WithEmbeddedStruct struct { TestStruct5WithEmbeddedStruct Field4 string `mapstructure:"field_4"` } +type TestStruct6WithEmbeddedStructAndSquashTag struct { + TestStruct5WithEmbeddedStructAndSquashTag `mapstructure:",squash"` + Field4 string `mapstructure:"field_4"` +} + func TestToMap(t *testing.T) { t.Run("generic", func(t *testing.T) { testStruct := TestStruct1{} @@ -176,8 +186,8 @@ func TestToMap(t *testing.T) { errortest.AssertError(t, FromMapToPointer[any](testMap, nil), commonerrors.ErrInvalid, commonerrors.ErrUndefined) errortest.AssertError(t, FromMapToPointer[*TestStruct3WithTime](testMap, nil), commonerrors.ErrInvalid, commonerrors.ErrUndefined) }) - t.Run("embedded struct mapping", func(t *testing.T) { - testStruct := TestStruct6WithEmbeddedStruct{} + t.Run("embedded struct mapping with `mapstructure:\",squash\"`", func(t *testing.T) { + testStruct := TestStruct6WithEmbeddedStructAndSquashTag{} require.NoError(t, faker.FakeData(&testStruct)) m := make(map[string]string) @@ -187,17 +197,43 @@ func TestToMap(t *testing.T) { m["field_4"] = faker.Word() // Should correctly set embedded struct fields from map m, even on nested embedded structs - err := FromMap[TestStruct6WithEmbeddedStruct](m, &testStruct) + err := FromMap[TestStruct6WithEmbeddedStructAndSquashTag](m, &testStruct) require.NoError(t, err) assert.Equal(t, testStruct.Field1, m["field_1"]) assert.Equal(t, testStruct.Field2, m["field_2"]) assert.Equal(t, testStruct.Field3, m["field_3"]) assert.Equal(t, testStruct.Field4, m["field_4"]) - structMap, err := ToMap[TestStruct6WithEmbeddedStruct](&testStruct) + structMap, err := ToMap[TestStruct6WithEmbeddedStructAndSquashTag](&testStruct) require.NoError(t, err) assert.Equal(t, m, structMap) + newStruct := TestStruct6WithEmbeddedStructAndSquashTag{} + require.NoError(t, FromMap[TestStruct6WithEmbeddedStructAndSquashTag](structMap, &newStruct)) + assert.Equal(t, testStruct, newStruct) + }) + t.Run("embedded struct mapping without `mapstructure:\",squash\"`", func(t *testing.T) { + testStruct := TestStruct6WithEmbeddedStruct{} + require.NoError(t, faker.FakeData(&testStruct)) + + m := make(map[string]string) + m["field_1"] = faker.Word() + m["field_2"] = faker.Word() + m["field_3"] = faker.Word() + m["field_4"] = faker.Word() + + // Should only set the parent struct fields from map m and ignore any embedded struct fields + err := FromMap[TestStruct6WithEmbeddedStruct](m, &testStruct) + require.NoError(t, err) + assert.NotEqual(t, testStruct.Field1, m["field_1"]) + assert.NotEqual(t, testStruct.Field2, m["field_2"]) + assert.NotEqual(t, testStruct.Field3, m["field_3"]) + assert.Equal(t, testStruct.Field4, m["field_4"]) + + structMap, err := ToMap[TestStruct6WithEmbeddedStruct](&testStruct) + require.NoError(t, err) + assert.NotEqual(t, m, structMap) + newStruct := TestStruct6WithEmbeddedStruct{} require.NoError(t, FromMap[TestStruct6WithEmbeddedStruct](structMap, &newStruct)) assert.Equal(t, testStruct, newStruct) From f2e3df370845a2df2fc76f0faf031c66607da37c Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Fri, 26 Sep 2025 13:34:21 +0100 Subject: [PATCH 5/6] Update secrets baseline --- .secrets.baseline | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 6949fcf414..5a03324b6e 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -66,6 +66,10 @@ { "path": "detect_secrets.filters.allowlist.is_line_allowlisted" }, + { + "path": "detect_secrets.filters.common.is_baseline_file", + "filename": ".secrets.baseline" + }, { "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", "min_level": 2 @@ -123,28 +127,28 @@ "filename": "utils/config/service_configuration_test.go", "hashed_secret": "ddcec2f503a5d58f432a0beee3fb9544fa581f54", "is_verified": false, - "line_number": 35 + "line_number": 37 }, { "type": "Secret Keyword", "filename": "utils/config/service_configuration_test.go", "hashed_secret": "7ca1cc114e7e5f955880bb96a5bf391b4dc20ab6", "is_verified": false, - "line_number": 533 + "line_number": 535 }, { "type": "Secret Keyword", "filename": "utils/config/service_configuration_test.go", "hashed_secret": "11519c144be4850d95b34220a40030cbd5a36b57", "is_verified": false, - "line_number": 628 + "line_number": 630 }, { "type": "Secret Keyword", "filename": "utils/config/service_configuration_test.go", "hashed_secret": "15fae91d8fa7f2c531c1cf3ddc745e1f4473c02d", "is_verified": false, - "line_number": 635 + "line_number": 637 } ], "utils/filesystem/filehash_test.go": [ @@ -272,5 +276,5 @@ } ] }, - "generated_at": "2025-08-08T23:03:50Z" + "generated_at": "2025-09-26T12:33:39Z" } From 063c5a061face168c1e4ccd15e3b5ffe769639be Mon Sep 17 00:00:00 2001 From: Kem Govender Date: Fri, 26 Sep 2025 13:44:17 +0100 Subject: [PATCH 6/6] Update comments --- utils/config/fixtures/env-test.env | 2 +- utils/config/service_configuration_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/config/fixtures/env-test.env b/utils/config/fixtures/env-test.env index 76b5c82ab9..eee71001a1 100644 --- a/utils/config/fixtures/env-test.env +++ b/utils/config/fixtures/env-test.env @@ -1,4 +1,4 @@ -# These env vars will map to a struct the embeds another struct with no mapstructure tag set (it will expect it's struct name as part of the env var) +# These env vars will map to a struct the embeds another struct with no mapstructure tag set (it will expect its struct name to be part of the env var) WITH_NO_TAG_TESTBASECFG_EMBEDDED1="embedded 1" WITH_NO_TAG_TESTBASECFG_EMBEDDED2="embedded 2" WITH_NO_TAG_NON_EMBEDDED1="non-embedded 1" diff --git a/utils/config/service_configuration_test.go b/utils/config/service_configuration_test.go index e9f13a9f43..5df642d2d8 100644 --- a/utils/config/service_configuration_test.go +++ b/utils/config/service_configuration_test.go @@ -860,7 +860,7 @@ func TestEmbeddedServiceConfigurationWithNoTagLoadFromFile(t *testing.T) { assert.Equal(t, "embedded 2", testEmbedded.Embedded2) } -// Config values loaded from file should be correctly mapped onto a struct that embeds another struct with a mapstructure tag set +// Nested config values loaded from file should be correctly mapped onto a struct that embeds another struct with a mapstructure tag set func TestEmbeddedServiceConfigurationWithTagLoadFromFile(t *testing.T) { os.Clearenv() session := viper.New() @@ -907,7 +907,7 @@ func TestEmbeddedServiceConfigurationWithSquashTagLoadFromNestedFile(t *testing. assert.NotEqual(t, "embedded 2", testEmbeddedWithSquashTag.Embedded2) } -// Config values loaded from file should be correctly mapped onto a struct that embeds another struct and maintains any defaults not overwritten +// Nested config values loaded from file should be correctly mapped onto a struct that embeds another struct and should maintain any defaults not overwritten func TestEmbeddedServiceConfigurationLoadFromFileWithDefaults(t *testing.T) { os.Clearenv() session := viper.New()