Skip to content

Commit

Permalink
Fix redis_sentinel configuration reading
Browse files Browse the repository at this point in the history
This fix solves these issues:
* The log messages were incorrectly using `redis-sentinel` instead of `redis_sentinel` when dumping the configuration in the logs, which was misleading.
* The configuration entries were not provided default values, preventing Viper from reading the associated environment variables
* The default configuration content was not tested
  • Loading branch information
sebmil-daily committed Sep 3, 2024
1 parent 9bca40c commit 742e424
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 31 deletions.
10 changes: 5 additions & 5 deletions config/backends.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,11 @@ type RedisSentinel struct {
}

func (cfg *RedisSentinel) validateAndLog() error {
log.Infof("config.backend.redis-sentinel.sentinel_addrs: %s", cfg.SentinelAddrs)
log.Infof("config.backend.redis-sentinel.master_name: %s", cfg.MasterName)
log.Infof("config.backend.redis-sentinel.db: %d", cfg.Db)
log.Infof("config.backend.redis-sentinel.tls.enabled: %t", cfg.TLS.Enabled)
log.Infof("config.backend.redis-sentinel.tls.insecure_skip_verify: %t", cfg.TLS.InsecureSkipVerify)
log.Infof("config.backend.redis_sentinel.sentinel_addrs: %s", cfg.SentinelAddrs)
log.Infof("config.backend.redis_sentinel.master_name: %s", cfg.MasterName)
log.Infof("config.backend.redis_sentinel.db: %d", cfg.Db)
log.Infof("config.backend.redis_sentinel.tls.enabled: %t", cfg.TLS.Enabled)
log.Infof("config.backend.redis_sentinel.tls.insecure_skip_verify: %t", cfg.TLS.InsecureSkipVerify)
return nil
}

Expand Down
6 changes: 6 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,12 @@ func setConfigDefaults(v *viper.Viper) {
v.SetDefault("backend.redis.expiration", utils.REDIS_DEFAULT_EXPIRATION_MINUTES)
v.SetDefault("backend.redis.tls.enabled", false)
v.SetDefault("backend.redis.tls.insecure_skip_verify", false)
v.SetDefault("backend.redis_sentinel.db", 0)
v.SetDefault("backend.redis_sentinel.master_name", "")
v.SetDefault("backend.redis_sentinel.password", "")
v.SetDefault("backend.redis_sentinel.sentinel_addrs", []string{})
v.SetDefault("backend.redis_sentinel.tls.enabled", false)
v.SetDefault("backend.redis_sentinel.tls.insecure_skip_verify", false)
v.SetDefault("backend.ignite.scheme", "")
v.SetDefault("backend.ignite.host", "")
v.SetDefault("backend.ignite.port", 0)
Expand Down
55 changes: 29 additions & 26 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ func TestCheckMetricsEnabled(t *testing.T) {
},
}

//Standard elements of the config.Metrics object are set so test cases only modify what's relevant to them
// Standard elements of the config.Metrics object are set so test cases only modify what's relevant to them
cfg := &Metrics{
Influx: InfluxMetrics{
Host: "http://fakeurl.com",
Expand All @@ -307,7 +307,7 @@ func TestCheckMetricsEnabled(t *testing.T) {
// logrus entries will be recorded to this `hook` object so we can compare and assert them
hook := testLogrus.NewGlobal()

//substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes
// substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes
defer func() { logrus.StandardLogger().ExitFunc = nil }()
var fatal bool
logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
Expand All @@ -321,7 +321,7 @@ func TestCheckMetricsEnabled(t *testing.T) {
cfg.Influx.Enabled = tc.influxEnabled
cfg.Prometheus.Enabled = tc.prometheusEnabled

//run test
// run test
cfg.validateAndLog()

// Assert logrus expected entries
Expand All @@ -337,7 +337,7 @@ func TestCheckMetricsEnabled(t *testing.T) {
// Assert log.Fatalf() was called or not
assert.Equal(t, tc.expectedError, fatal, "Test case %d failed.", i+1)

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand Down Expand Up @@ -447,7 +447,7 @@ func TestEnabledFlagGetsModified(t *testing.T) {
// logrus entries will be recorded to this `hook` object so we can compare and assert them
hook := testLogrus.NewGlobal()

//substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes
// substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes
defer func() { logrus.StandardLogger().ExitFunc = nil }()
logrus.StandardLogger().ExitFunc = func(int) {}

Expand All @@ -468,14 +468,14 @@ func TestEnabledFlagGetsModified(t *testing.T) {
},
}

//run test
// run test
metricsCfg.validateAndLog()

// Assert `Enabled` flags value
assert.Equal(t, tc.out.expectedInfluxEnabled, metricsCfg.Influx.Enabled, "Test case %d failed. `cfg.Influx.Enabled` carries wrong value.", i+1)
assert.Equal(t, tc.out.expectedprometheusEnabled, metricsCfg.Prometheus.Enabled, "Test case %d failed. `cfg.Prometheus.Enabled` carries wrong value.", i+1)

//Reset log after every test
// Reset log after every test
hook.Reset()
}
}
Expand All @@ -493,7 +493,7 @@ func TestInfluxValidateAndLog(t *testing.T) {
description string
// In
influxConfig *InfluxMetrics
//out
// out
expectError bool
expectedLogInfo []logComponents
}
Expand Down Expand Up @@ -598,7 +598,7 @@ func TestInfluxValidateAndLog(t *testing.T) {
},
}

//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
var fatal bool
logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
Expand All @@ -607,7 +607,7 @@ func TestInfluxValidateAndLog(t *testing.T) {
// Reset the fatal flag to false every test
fatal = false

//run test
// run test
tc.influxConfig.validateAndLog()

// Assert logrus expected entries
Expand All @@ -623,7 +623,7 @@ func TestInfluxValidateAndLog(t *testing.T) {
// Assert log.Fatalf() was called or not
assert.Equal(t, tc.expectError, fatal)

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand All @@ -639,7 +639,7 @@ func TestPrometheusValidateAndLog(t *testing.T) {
description string
// In
prometheusConfig *PrometheusMetrics
//out
// out
expectError bool
expectedLogInfo []logComponents
}
Expand Down Expand Up @@ -768,7 +768,7 @@ func TestPrometheusValidateAndLog(t *testing.T) {
// logrus entries will be recorded to this `hook` object so we can compare and assert them
hook := testLogrus.NewGlobal()

//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
var fatal bool
logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
Expand All @@ -777,7 +777,7 @@ func TestPrometheusValidateAndLog(t *testing.T) {
// Reset the fatal flag to false every test
fatal = false

//run test
// run test
tc.prometheusConfig.validateAndLog()

// Assert logrus expected entries
Expand All @@ -793,7 +793,7 @@ func TestPrometheusValidateAndLog(t *testing.T) {
// Assert log.Fatalf() was called or not
assert.Equal(t, tc.expectError, fatal)

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand Down Expand Up @@ -869,7 +869,7 @@ func TestRequestLimitsValidateAndLog(t *testing.T) {
},
}

//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
var fatal bool
logrus.StandardLogger().ExitFunc = func(int) { fatal = true }
Expand Down Expand Up @@ -897,7 +897,7 @@ func TestRequestLimitsValidateAndLog(t *testing.T) {
}
assert.Len(t, tc.expectedLogInfo, logEntryCount, tc.description)

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand Down Expand Up @@ -953,7 +953,7 @@ func TestCompressionValidateAndLog(t *testing.T) {
},
}

//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
logrus.StandardLogger().ExitFunc = func(int) {}

Expand All @@ -969,7 +969,7 @@ func TestCompressionValidateAndLog(t *testing.T) {
}
}

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand Down Expand Up @@ -1029,12 +1029,12 @@ func TestNewConfigFromFile(t *testing.T) {
},
}

//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
logrus.StandardLogger().ExitFunc = func(int) {}

for _, tc := range testCases {
//run test
// run test
actualCfg := NewConfig(tc.inConfigFileName)

// Assert logrus expected entries
Expand All @@ -1047,7 +1047,7 @@ func TestNewConfigFromFile(t *testing.T) {

assert.Equal(t, tc.expectedConfig, actualCfg, "Expected Configuration instance does not match. Test desc:%s", tc.description)

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand All @@ -1056,7 +1056,7 @@ func TestNewConfigFromFile(t *testing.T) {
func TestConfigurationValidateAndLog(t *testing.T) {
// logrus entries will be recorded to this `hook` object so we can compare and assert them
hook := testLogrus.NewGlobal()
//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
logrus.StandardLogger().ExitFunc = func(int) {}

Expand Down Expand Up @@ -1094,7 +1094,7 @@ func TestConfigurationValidateAndLog(t *testing.T) {
}
}

//Reset log
// Reset log
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand Down Expand Up @@ -1137,7 +1137,7 @@ func TestRoutesValidateAndLog(t *testing.T) {
},
}

//substitute logger exit function so execution doesn't get interrupted
// substitute logger exit function so execution doesn't get interrupted
defer func() { logrus.StandardLogger().ExitFunc = nil }()
logrus.StandardLogger().ExitFunc = func(int) {}

Expand All @@ -1153,7 +1153,7 @@ func TestRoutesValidateAndLog(t *testing.T) {
}
}

//Reset log after every test and assert successful reset
// Reset log after every test and assert successful reset
hook.Reset()
assert.Nil(t, hook.LastEntry())
}
Expand Down Expand Up @@ -1204,6 +1204,9 @@ func getExpectedDefaultConfig() Configuration {
Redis: Redis{
ExpirationMinutes: utils.REDIS_DEFAULT_EXPIRATION_MINUTES,
},
RedisSentinel: RedisSentinel{
SentinelAddrs: []string{},
},
Ignite: Ignite{
Headers: map[string]string{},
},
Expand Down

0 comments on commit 742e424

Please sign in to comment.