Skip to content

Commit

Permalink
fix(configuration): optional value not treated as optional (#5853)
Browse files Browse the repository at this point in the history
Fixes a configuration validation process for redis where the redis port was meant to be optional but was not treated as optional.

Signed-off-by: Paul Calabro <585059+paulcalabro@users.noreply.github.com>
  • Loading branch information
paulcalabro committed Aug 18, 2023
1 parent f807ea1 commit 5edd5fc
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 4 deletions.
1 change: 1 addition & 0 deletions internal/configuration/schema/session.go
Expand Up @@ -77,6 +77,7 @@ var DefaultSessionConfiguration = SessionConfiguration{

// DefaultRedisConfiguration is the default redis configuration.
var DefaultRedisConfiguration = RedisSessionConfiguration{
Port: 6379,
TLS: &TLSConfig{
MinimumVersion: TLSVersion{Value: tls.VersionTLS12},
},
Expand Down
6 changes: 4 additions & 2 deletions internal/configuration/validator/session.go
Expand Up @@ -218,7 +218,9 @@ func validateRedis(config *schema.SessionConfiguration, validator *schema.Struct

validateRedisCommon(config, validator)

if !path.IsAbs(config.Redis.Host) && (config.Redis.Port < 1 || config.Redis.Port > 65535) {
if config.Redis.Port == 0 {
config.Redis.Port = schema.DefaultRedisConfiguration.Port
} else if !path.IsAbs(config.Redis.Host) && (config.Redis.Port < 1 || config.Redis.Port > 65535) {
validator.Push(fmt.Errorf(errFmtSessionRedisPortRange, config.Redis.Port))
}

Expand All @@ -234,7 +236,7 @@ func validateRedisSentinel(config *schema.SessionConfiguration, validator *schem

if config.Redis.Port == 0 {
config.Redis.Port = 26379
} else if config.Redis.Port < 0 || config.Redis.Port > 65535 {
} else if config.Redis.Port < 1 || config.Redis.Port > 65535 {
validator.Push(fmt.Errorf(errFmtSessionRedisPortRange, config.Redis.Port))
}

Expand Down
29 changes: 27 additions & 2 deletions internal/configuration/validator/session_test.go
Expand Up @@ -299,7 +299,7 @@ func TestShouldRaiseErrorWhenRedisIsUsedAndSecretNotSet(t *testing.T) {
assert.EqualError(t, validator.Errors()[0], fmt.Sprintf(errFmtSessionSecretRequired, "redis"))
}

func TestShouldRaiseErrorWhenRedisHasHostnameButNoPort(t *testing.T) {
func TestShouldNotRaiseErrorsAndSetDefaultPortWhenRedisPortBlank(t *testing.T) {
validator := schema.NewStructValidator()
config := newDefaultSessionConfig()

Expand All @@ -318,9 +318,34 @@ func TestShouldRaiseErrorWhenRedisHasHostnameButNoPort(t *testing.T) {

ValidateSession(&config, validator)

assert.False(t, validator.HasWarnings())
assert.False(t, validator.HasErrors())

assert.Equal(t, 6379, config.Redis.Port)
}

func TestShouldRaiseErrorWhenRedisPortInvalid(t *testing.T) {
validator := schema.NewStructValidator()
config := newDefaultSessionConfig()

ValidateSession(&config, validator)

assert.Len(t, validator.Errors(), 0)
validator.Clear()

config = newDefaultSessionConfig()

// Set redis config because password must be set only when redis is used.
config.Redis = &schema.RedisSessionConfiguration{
Host: "redis.localhost",
Port: -1,
}

ValidateSession(&config, validator)

assert.False(t, validator.HasWarnings())
assert.Len(t, validator.Errors(), 1)
assert.EqualError(t, validator.Errors()[0], "session: redis: option 'port' must be between 1 and 65535 but it's configured as '0'")
assert.EqualError(t, validator.Errors()[0], "session: redis: option 'port' must be between 1 and 65535 but it's configured as '-1'")
}

func TestShouldRaiseOneErrorWhenRedisHighAvailabilityHasNodesWithNoHost(t *testing.T) {
Expand Down

0 comments on commit 5edd5fc

Please sign in to comment.