From 91e3bfec92ae133ad13b91838cf16e3b85d1a7cd Mon Sep 17 00:00:00 2001 From: evanebb Date: Tue, 20 May 2025 16:46:17 +0200 Subject: [PATCH 1/3] Inline env variables if the underlying struct is YAML inlined Signed-off-by: evanebb --- configuration/parser.go | 20 ++++++++++++++++ configuration/parser_test.go | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/configuration/parser.go b/configuration/parser.go index 1a9201986..562067683 100644 --- a/configuration/parser.go +++ b/configuration/parser.go @@ -4,6 +4,7 @@ import ( "fmt" "os" "reflect" + "slices" "sort" "strconv" "strings" @@ -205,6 +206,25 @@ func (p *Parser) overwriteStruct(v reflect.Value, fullpath string, path []string byUpperCase := make(map[string]int) for i := 0; i < v.NumField(); i++ { sf := v.Type().Field(i) + + // For fields inlined in the YAML configuration file, the environment variables also need to be inlined + // Example struct tag for inlined field: `yaml:",inline"` + yamlTag := sf.Tag.Get("yaml") + split := strings.Split(yamlTag, ",") + // Skip the first entry, which is the field name + isInlined := slices.Contains(split[1:], "inline") + + if isInlined && sf.Type.Kind() == reflect.Struct { + // Inlined struct, check whether the env variable corresponds to a field inside this struct + // Maps could also be inlined, but since we don't need it right now it is not supported + inlined := v.Field(i) + for j := 0; j < inlined.NumField(); j++ { + if strings.ToUpper(inlined.Type().Field(j).Name) == path[0] { + return p.overwriteFields(inlined, fullpath, path, payload) + } + } + } + upper := strings.ToUpper(sf.Name) if _, present := byUpperCase[upper]; present { panic(fmt.Sprintf("field name collision in configuration object: %s", sf.Name)) diff --git a/configuration/parser_test.go b/configuration/parser_test.go index 636f3291c..637a20075 100644 --- a/configuration/parser_test.go +++ b/configuration/parser_test.go @@ -11,12 +11,18 @@ type localConfiguration struct { Version Version `yaml:"version"` Log *Log `yaml:"log"` Notifications []Notif `yaml:"notifications,omitempty"` + Inlined Inlined `yaml:",inline"` } type Notif struct { Name string `yaml:"name"` } +type Inlined struct { + FirstValue string `yaml:"firstValue"` + SecondValue string `yaml:"secondValue"` +} + var expectedConfig = localConfiguration{ Version: "0.1", Log: &Log{ @@ -89,3 +95,41 @@ func TestParseOverwriteUnininitializedPoiner(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedConfig, config) } + +const testConfig3 = `version: "0.1" +log: + formatter: "text"` + +func TestParseInlinedStruct(t *testing.T) { + config := localConfiguration{} + + expected := localConfiguration{ + Version: "0.1", + Log: &Log{ + Formatter: "text", + }, + Inlined: Inlined{ + FirstValue: "foo", + SecondValue: "bar", + }, + } + + // Test without inlined struct name in the env variable name + t.Setenv("REGISTRY_FIRSTVALUE", "foo") + // Test with the inlined struct name in the env variable name, for backward compatibility + t.Setenv("REGISTRY_INLINED_SECONDVALUE", "bar") + + p := NewParser("registry", []VersionedParseInfo{ + { + Version: "0.1", + ParseAs: reflect.TypeOf(config), + ConversionFunc: func(c interface{}) (interface{}, error) { + return c, nil + }, + }, + }) + + err := p.Parse([]byte(testConfig3), &config) + require.NoError(t, err) + require.Equal(t, expected, config) +} From b65cccafa2669dc3d816d40c3f4c84ee03ee5079 Mon Sep 17 00:00:00 2001 From: evanebb Date: Fri, 23 May 2025 12:20:13 +0200 Subject: [PATCH 2/3] Add test for inlined structs to configuration package Signed-off-by: evanebb --- configuration/configuration_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/configuration/configuration_test.go b/configuration/configuration_test.go index b0fe3c08e..514d8132a 100644 --- a/configuration/configuration_test.go +++ b/configuration/configuration_test.go @@ -456,6 +456,21 @@ func (suite *ConfigSuite) TestParseEnvMany() { suite.Require().NoError(err) } +// TestParseEnvInlinedStruct tests whether environment variables are properly matched to fields in inlined structs. +func (suite *ConfigSuite) TestParseEnvInlinedStruct() { + suite.expectedConfig.Redis.Options.Username = "bob" + suite.expectedConfig.Redis.Options.Password = "password123" + + // Test without inlined struct name in the env variable name + suite.T().Setenv("REGISTRY_REDIS_USERNAME", "bob") + // Test with the inlined struct name in the env variable name, for backward compatibility + suite.T().Setenv("REGISTRY_REDIS_OPTIONS_PASSWORD", "password123") + + config, err := Parse(bytes.NewReader([]byte(configYamlV0_1))) + suite.Require().NoError(err) + suite.Require().Equal(suite.expectedConfig, config) +} + func checkStructs(tt *testing.T, t reflect.Type, structsChecked map[string]struct{}) { tt.Helper() From fa24405ad9fd373821073b4dfa5210575379a8de Mon Sep 17 00:00:00 2001 From: evanebb Date: Sat, 24 May 2025 16:58:14 +0200 Subject: [PATCH 3/3] Apply code review suggestions Signed-off-by: evanebb --- configuration/parser.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/configuration/parser.go b/configuration/parser.go index 562067683..d94ce3156 100644 --- a/configuration/parser.go +++ b/configuration/parser.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "reflect" - "slices" "sort" "strconv" "strings" @@ -209,17 +208,13 @@ func (p *Parser) overwriteStruct(v reflect.Value, fullpath string, path []string // For fields inlined in the YAML configuration file, the environment variables also need to be inlined // Example struct tag for inlined field: `yaml:",inline"` - yamlTag := sf.Tag.Get("yaml") - split := strings.Split(yamlTag, ",") - // Skip the first entry, which is the field name - isInlined := slices.Contains(split[1:], "inline") - - if isInlined && sf.Type.Kind() == reflect.Struct { + _, yamlOpts, _ := strings.Cut(sf.Tag.Get("yaml"), ",") + if yamlOpts == "inline" && sf.Type.Kind() == reflect.Struct { // Inlined struct, check whether the env variable corresponds to a field inside this struct // Maps could also be inlined, but since we don't need it right now it is not supported inlined := v.Field(i) - for j := 0; j < inlined.NumField(); j++ { - if strings.ToUpper(inlined.Type().Field(j).Name) == path[0] { + for j := range inlined.NumField() { + if strings.EqualFold(inlined.Type().Field(j).Name, path[0]) { return p.overwriteFields(inlined, fullpath, path, payload) } }