Skip to content

Commit bb1889b

Browse files
sinadarbouysinadarbouy
authored
Fix/env vars expect lower case letters (#587)
* Added function to Convert Keys To Lowercase * added check config block to validation * convert all refrereced configblock to lowercase * fixed lint issues * Fix error wrapping to create new instances for each error * added testcases for LoadEnvVariables * added testcase for ConvertKeysToLowercase * refactor lowerKey to lowercaseKey * refactor active-writes to writes and standby-reads to reads * set env var for test cases from cons * fixed lint issues
1 parent 0127264 commit bb1889b

File tree

10 files changed

+239
-32
lines changed

10 files changed

+239
-32
lines changed

‎cmd/configs.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ func lintConfig(fileType configFileType, configFile string) *gerr.GatewayDError
8080
if err := conf.LoadGlobalConfigFile(context.TODO()); err != nil {
8181
return err
8282
}
83+
if err := conf.ConvertKeysToLowercase(context.TODO()); err != nil {
84+
return err
85+
}
8386
if err := conf.UnmarshalGlobalConfig(context.TODO()); err != nil {
8487
return err
8588
}

‎cmd/testdata/gatewayd.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ metrics:
1818

1919
clients:
2020
default:
21-
activeWrites:
21+
writes:
2222
address: localhost:5432
2323
network: tcp
2424
tcpKeepAlive: False
@@ -51,15 +51,15 @@ clients:
5151

5252
pools:
5353
default:
54-
activeWrites:
54+
writes:
5555
size: 10
5656
test:
5757
write:
5858
size: 10
5959

6060
proxies:
6161
default:
62-
activeWrites:
62+
writes:
6363
healthCheckPeriod: 60s # duration
6464
test:
6565
write:

‎cmd/testdata/gatewayd_tls.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ metrics:
1212

1313
clients:
1414
default:
15-
activeWrites:
15+
writes:
1616
address: localhost:5432
1717
network: tcp
1818
tcpKeepAlive: False
@@ -29,12 +29,12 @@ clients:
2929

3030
pools:
3131
default:
32-
activeWrites:
32+
writes:
3333
size: 10
3434

3535
proxies:
3636
default:
37-
activeWrites:
37+
writes:
3838
healthCheckPeriod: 60s # duration
3939

4040
servers:

‎config/config.go‎

Lines changed: 158 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package config
22

33
import (
44
"context"
5+
"encoding/json"
56
goerrors "errors"
67
"fmt"
78
"log"
@@ -82,10 +83,12 @@ func (c *Config) InitConfig(ctx context.Context) *gerr.GatewayDError {
8283
if err := c.UnmarshalPluginConfig(newCtx); err != nil {
8384
return err
8485
}
85-
8686
if err := c.LoadGlobalConfigFile(newCtx); err != nil {
8787
return err
8888
}
89+
if err := c.ConvertKeysToLowercase(newCtx); err != nil {
90+
return err
91+
}
8992
if err := c.ValidateGlobalConfig(newCtx); err != nil {
9093
return err
9194
}
@@ -465,6 +468,105 @@ func (c *Config) MergeGlobalConfig(
465468
return nil
466469
}
467470

471+
// convertMapKeysToLowercase converts all keys in a map to lowercase.
472+
//
473+
// Parameters:
474+
// - m: A map with string keys to be converted.
475+
//
476+
// Returns:
477+
// - map[string]*T: A new map with lowercase keys.
478+
func convertMapKeysToLowercase[T any](m map[string]*T) map[string]*T {
479+
newMap := make(map[string]*T)
480+
for k, v := range m {
481+
lowercaseKey := strings.ToLower(k)
482+
newMap[lowercaseKey] = v
483+
}
484+
return newMap
485+
}
486+
487+
// convertNestedMapKeysToLowercase converts all keys in a nested map structure to lowercase.
488+
//
489+
// Parameters:
490+
// - m: A nested map with string keys to be converted.
491+
//
492+
// Returns:
493+
// - map[string]map[string]*T: A new nested map with lowercase keys.
494+
func convertNestedMapKeysToLowercase[T any](m map[string]map[string]*T) map[string]map[string]*T {
495+
newMap := make(map[string]map[string]*T)
496+
for k, v := range m {
497+
lowercaseKey := strings.ToLower(k)
498+
newMap[lowercaseKey] = convertMapKeysToLowercase(v)
499+
}
500+
return newMap
501+
}
502+
503+
// ConvertKeysToLowercase converts all keys in the global configuration to lowercase.
504+
// It unmarshals the configuration data into a GlobalConfig struct, then recursively converts
505+
// all map keys to lowercase.
506+
//
507+
// Parameters:
508+
// - ctx (context.Context): The context for tracing and cancellation, used for monitoring
509+
// and propagating execution state.
510+
//
511+
// Returns:
512+
// - *gerr.GatewayDError: An error if unmarshalling fails, otherwise nil.
513+
func (c *Config) ConvertKeysToLowercase(ctx context.Context) *gerr.GatewayDError {
514+
_, span := otel.Tracer(TracerName).Start(ctx, "Validate global config")
515+
516+
defer span.End()
517+
518+
var globalConfig GlobalConfig
519+
if err := c.GlobalKoanf.Unmarshal("", &globalConfig); err != nil {
520+
span.RecordError(err)
521+
return gerr.ErrValidationFailed.Wrap(
522+
fmt.Errorf("failed to unmarshal global configuration: %w", err))
523+
}
524+
525+
globalConfig.Loggers = convertMapKeysToLowercase(globalConfig.Loggers)
526+
globalConfig.Clients = convertNestedMapKeysToLowercase(globalConfig.Clients)
527+
globalConfig.Pools = convertNestedMapKeysToLowercase(globalConfig.Pools)
528+
globalConfig.Proxies = convertNestedMapKeysToLowercase(globalConfig.Proxies)
529+
globalConfig.Servers = convertMapKeysToLowercase(globalConfig.Servers)
530+
globalConfig.Metrics = convertMapKeysToLowercase(globalConfig.Metrics)
531+
532+
// Convert the globalConfig back to a map[string]interface{}
533+
configMap, err := structToMap(globalConfig)
534+
if err != nil {
535+
span.RecordError(err)
536+
return gerr.ErrValidationFailed.Wrap(
537+
fmt.Errorf("failed to convert global configuration to map: %w", err))
538+
}
539+
540+
// Create a new koanf instance and load the updated map
541+
newKoanf := koanf.New(".")
542+
if err := newKoanf.Load(confmap.Provider(configMap, "."), nil); err != nil {
543+
span.RecordError(err)
544+
return gerr.ErrValidationFailed.Wrap(
545+
fmt.Errorf("failed to load updated configuration into koanf: %w", err))
546+
}
547+
548+
// Update the GlobalKoanf with the new instance
549+
c.GlobalKoanf = newKoanf
550+
// Update the Global with the new instance
551+
c.Global = globalConfig
552+
553+
return nil
554+
}
555+
556+
// structToMap converts a given struct to a map[string]interface{}.
557+
func structToMap(v interface{}) (map[string]interface{}, error) {
558+
var result map[string]interface{}
559+
data, err := json.Marshal(v)
560+
if err != nil {
561+
return nil, fmt.Errorf("failed to marshal struct: %w", err)
562+
}
563+
err = json.Unmarshal(data, &result)
564+
if err != nil {
565+
return nil, fmt.Errorf("failed to unmarshal data into map: %w", err)
566+
}
567+
return result, nil
568+
}
569+
468570
func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
469571
_, span := otel.Tracer(TracerName).Start(ctx, "Validate global config")
470572

@@ -487,6 +589,11 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
487589
span.RecordError(err)
488590
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
489591
}
592+
if configGroup != strings.ToLower(configGroup) {
593+
err := fmt.Errorf(`"logger.%s" is not lowercase`, configGroup)
594+
span.RecordError(err)
595+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
596+
}
490597
}
491598

492599
if len(globalConfig.Loggers) > 1 {
@@ -499,6 +606,11 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
499606
span.RecordError(err)
500607
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
501608
}
609+
if configGroup != strings.ToLower(configGroup) {
610+
err := fmt.Errorf(`"metrics.%s" is not lowercase`, configGroup)
611+
span.RecordError(err)
612+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
613+
}
502614
}
503615

504616
if len(globalConfig.Metrics) > 1 {
@@ -510,10 +622,20 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
510622
if _, ok := clientConfigGroups[configGroupName]; !ok {
511623
clientConfigGroups[configGroupName] = make(map[string]bool)
512624
}
513-
for configGroup := range configGroups {
514-
clientConfigGroups[configGroupName][configGroup] = true
515-
if globalConfig.Clients[configGroupName][configGroup] == nil {
516-
err := fmt.Errorf("\"clients.%s\" is nil or empty", configGroup)
625+
if configGroupName != strings.ToLower(configGroupName) {
626+
err := fmt.Errorf(`"clients.%s" is not lowercase`, configGroupName)
627+
span.RecordError(err)
628+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
629+
}
630+
for configBlockName := range configGroups {
631+
clientConfigGroups[configGroupName][configBlockName] = true
632+
if globalConfig.Clients[configGroupName][configBlockName] == nil {
633+
err := fmt.Errorf(`"clients.%s" is nil or empty`, configBlockName)
634+
span.RecordError(err)
635+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
636+
}
637+
if configBlockName != strings.ToLower(configBlockName) {
638+
err := fmt.Errorf(`"clients.%s.%s" is not lowercase`, configGroupName, configBlockName)
517639
span.RecordError(err)
518640
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
519641
}
@@ -530,6 +652,11 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
530652
span.RecordError(err)
531653
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
532654
}
655+
if configGroup != strings.ToLower(configGroup) {
656+
err := fmt.Errorf(`"pools.%s" is not lowercase`, configGroup)
657+
span.RecordError(err)
658+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
659+
}
533660
}
534661

535662
if len(globalConfig.Pools) > 1 {
@@ -542,6 +669,11 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
542669
span.RecordError(err)
543670
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
544671
}
672+
if configGroup != strings.ToLower(configGroup) {
673+
err := fmt.Errorf(`"proxies.%s" is not lowercase`, configGroup)
674+
span.RecordError(err)
675+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
676+
}
545677
}
546678

547679
if len(globalConfig.Proxies) > 1 {
@@ -554,6 +686,11 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
554686
span.RecordError(err)
555687
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
556688
}
689+
if configGroup != strings.ToLower(configGroup) {
690+
err := fmt.Errorf(`"servers.%s" is not lowercase`, configGroup)
691+
span.RecordError(err)
692+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
693+
}
557694
}
558695

559696
if len(globalConfig.Servers) > 1 {
@@ -570,9 +707,14 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
570707

571708
// Check if all proxies are referenced in client configuration
572709
for configGroupName, configGroups := range globalConfig.Proxies {
573-
for configGroup := range configGroups {
574-
if !clientConfigGroups[configGroupName][configGroup] {
575-
err := fmt.Errorf(`"proxies.%s" not referenced in client configuration`, configGroup)
710+
for configBlockName := range configGroups {
711+
if !clientConfigGroups[configGroupName][configBlockName] {
712+
err := fmt.Errorf(`"proxies.%s.%s" not referenced in client configuration`, configGroupName, configBlockName)
713+
span.RecordError(err)
714+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
715+
}
716+
if configBlockName != strings.ToLower(configBlockName) {
717+
err := fmt.Errorf(`"proxies.%s.%s" is not lowercase`, configGroupName, configBlockName)
576718
span.RecordError(err)
577719
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
578720
}
@@ -581,9 +723,14 @@ func (c *Config) ValidateGlobalConfig(ctx context.Context) *gerr.GatewayDError {
581723

582724
// Check if all pools are referenced in client configuration
583725
for configGroupName, configGroups := range globalConfig.Pools {
584-
for configGroup := range configGroups {
585-
if !clientConfigGroups[configGroupName][configGroup] {
586-
err := fmt.Errorf(`"pools.%s" not referenced in client configuration`, configGroup)
726+
for configBlockName := range configGroups {
727+
if !clientConfigGroups[configGroupName][configBlockName] {
728+
err := fmt.Errorf(`"pools.%s.%s" not referenced in client configuration`, configGroupName, configBlockName)
729+
span.RecordError(err)
730+
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
731+
}
732+
if configBlockName != strings.ToLower(configBlockName) {
733+
err := fmt.Errorf(`"pools.%s.%s" is not lowercase`, configGroupName, configBlockName)
587734
span.RecordError(err)
588735
errors = append(errors, gerr.ErrValidationFailed.Wrap(err))
589736
}

‎config/config_test.go‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,13 +188,50 @@ func pluginDefaultPolicyOverwrite(t *testing.T) {
188188
assert.Equal(t, "test", config.Plugin.DefaultPolicy)
189189
}
190190

191+
// clientNetworkOverwrite sets the environment variable for client network configuration
192+
// and verifies that the configuration is correctly loaded with the expected value.
193+
func clientNetworkOverwrite(t *testing.T) {
194+
t.Helper()
195+
ctx := context.Background()
196+
// Convert to uppercase
197+
upperDefaultGroup := strings.ToUpper(Default)
198+
upperDefaultBlock := strings.ToUpper(DefaultConfigurationBlock)
199+
200+
// Format environment variable name
201+
envVarName := fmt.Sprintf("GATEWAYD_CLIENTS_%s_%s_NETWORK", upperDefaultGroup, upperDefaultBlock)
202+
203+
// Set the environment variable
204+
t.Setenv(envVarName, "udp")
205+
config := initializeConfig(ctx, t)
206+
assert.Equal(t, "udp", config.Global.Clients[Default][DefaultConfigurationBlock].Network)
207+
}
208+
209+
// serverNetworkOverwrite sets the environment variable for server network configuration
210+
// and verifies that the configuration is correctly loaded with the expected value.
211+
func serverNetworkOverwrite(t *testing.T) {
212+
t.Helper()
213+
ctx := context.Background()
214+
// Convert to uppercase
215+
upperDefaultGroup := strings.ToUpper(Default)
216+
217+
// Format environment variable name
218+
envVarName := fmt.Sprintf("GATEWAYD_SERVERS_%s_NETWORK", upperDefaultGroup)
219+
220+
// Set the environment variable
221+
t.Setenv(envVarName, "udp")
222+
config := initializeConfig(ctx, t)
223+
assert.Equal(t, "udp", config.Global.Servers[Default].Network)
224+
}
225+
191226
// TestLoadEnvVariables runs a suite of tests to verify that environment variables are correctly
192227
// loaded into the configuration. Each test scenario sets a specific environment variable and
193228
// checks if the configuration reflects the expected value.
194229
func TestLoadEnvVariables(t *testing.T) {
195230
scenarios := map[string]func(t *testing.T){
196231
"serverLoadBalancerStrategyOverwrite": ServerLoadBalancerStrategyOverwrite,
197232
"pluginLocalPathOverwrite": pluginDefaultPolicyOverwrite,
233+
"ClientNetworkOverwrite": clientNetworkOverwrite,
234+
"ServerNetworkOverwrite": serverNetworkOverwrite,
198235
}
199236

200237
for scenario, fn := range scenarios {
@@ -203,3 +240,20 @@ func TestLoadEnvVariables(t *testing.T) {
203240
})
204241
}
205242
}
243+
244+
// TestConvertKeysToLowercaseSuccess verifies that after calling ConvertKeysToLowercase,
245+
// all keys in the config.Global.Clients map are converted to lowercase.
246+
func TestConvertKeysToLowercaseSuccess(t *testing.T) {
247+
ctx := context.Background()
248+
config := NewConfig(ctx,
249+
Config{GlobalConfigFile: parentDir + GlobalConfigFilename, PluginConfigFile: parentDir + PluginsConfigFilename})
250+
251+
err := config.ConvertKeysToLowercase(ctx)
252+
require.Nil(t, err)
253+
for configurationGroupName, configurationGroup := range config.Global.Clients {
254+
assert.Equal(t, configurationGroupName, strings.ToLower(configurationGroupName))
255+
for configuraionBlockName := range configurationGroup {
256+
assert.Equal(t, configuraionBlockName, strings.ToLower(configuraionBlockName))
257+
}
258+
}
259+
}

‎config/constants.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const (
3535
const (
3636
// Config constants.
3737
Default = "default"
38-
DefaultConfigurationBlock = "activeWrites"
38+
DefaultConfigurationBlock = "writes"
3939
EnvPrefix = "GATEWAYD_"
4040
TracerName = "gatewayd"
4141
GlobalConfigFilename = "gatewayd.yaml"

0 commit comments

Comments
 (0)