From 4a4eb651df218f6925854245ec1334c10389bab4 Mon Sep 17 00:00:00 2001 From: Konstantinos Christofilos Date: Tue, 9 Feb 2021 18:24:09 +0100 Subject: [PATCH] Password support for Redis (#1703) * - Support for password enabled Redis servers in rate limit - Set Redis password using environment variable Signed-off-by: Konstantinos Christofilos * Set Redis password from config file Signed-off-by: Konstantinos Christofilos --- config/config.go | 16 ++++++- config/config_test.go | 53 ++++++++++++++++++++++- config/test.yaml | 1 + ratelimit/redis.go | 3 ++ ratelimit/redis_test.go | 95 +++++++++++++++++++++++++++++++++++++---- skipper.go | 2 + 6 files changed, 159 insertions(+), 11 deletions(-) diff --git a/config/config.go b/config/config.go index 030cf372d8..550dc9f30d 100644 --- a/config/config.go +++ b/config/config.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "net/http" + "os" "sort" "strconv" "strings" @@ -208,6 +209,7 @@ type Config struct { EnableSwarm bool `yaml:"enable-swarm"` // redis based SwarmRedisURLs *listFlag `yaml:"swarm-redis-urls"` + SwarmRedisPassword string `yaml:"swarm-redis-password"` SwarmRedisDialTimeout time.Duration `yaml:"swarm-redis-dial-timeout"` SwarmRedisReadTimeout time.Duration `yaml:"swarm-redis-read-timeout"` SwarmRedisWriteTimeout time.Duration `yaml:"swarm-redis-write-timeout"` @@ -443,7 +445,7 @@ const ( swarmPortUsage = "swarm port to use to communicate with our peers" swarmMaxMessageBufferUsage = "swarm max message buffer size to use for member list messages" swarmLeaveTimeoutUsage = "swarm leave timeout to use for leaving the memberlist on timeout" - swarmRedisURLsUsage = "Redis URLs as comma separated list, used for building a swarm, for example in redis based cluster ratelimits" + swarmRedisURLsUsage = "Redis URLs as comma separated list, used for building a swarm, for example in redis based cluster ratelimits.\nUse " + redisPasswordEnv + " environment variable or 'swarm-redis-password' key in config file to set redis password" swarmStaticSelfUsage = "set static swarm self node, for example 127.0.0.1:9001" swarmStaticOtherUsage = "set static swarm all nodes, for example 127.0.0.1:9002,127.0.0.1:9003" swarmRedisDialTimeoutUsage = "set redis client dial timeout" @@ -452,6 +454,9 @@ const ( swarmRedisPoolTimeoutUsage = "set redis get connection from pool timeout" swarmRedisMaxConnsUsage = "set max number of connections to redis" swarmRedisMinConnsUsage = "set min number of connections to redis" + + // environment keys: + redisPasswordEnv = "SWARM_REDIS_PASSWORD" ) func NewConfig() *Config { @@ -735,6 +740,7 @@ func (c *Config) Parse() error { c.Certificates = certificates } + c.parseEnv() return nil } @@ -913,6 +919,7 @@ func (c *Config) ToOptions() skipper.Options { EnableSwarm: c.EnableSwarm, // redis based SwarmRedisURLs: c.SwarmRedisURLs.values, + SwarmRedisPassword: c.SwarmRedisPassword, SwarmRedisDialTimeout: c.SwarmRedisDialTimeout, SwarmRedisReadTimeout: c.SwarmRedisReadTimeout, SwarmRedisWriteTimeout: c.SwarmRedisWriteTimeout, @@ -996,3 +1003,10 @@ func (c *Config) parseHistogramBuckets() ([]float64, error) { sort.Float64s(result) return result, nil } + +func (c *Config) parseEnv() { + // Set Redis password from environment variable if not set earlier (configuration file) + if c.SwarmRedisPassword == "" { + c.SwarmRedisPassword = os.Getenv(redisPasswordEnv) + } +} diff --git a/config/config_test.go b/config/config_test.go index 7b0906fde5..c1b1ffaaa1 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -12,9 +12,57 @@ import ( "github.com/google/go-cmp/cmp" ) -func Test_NewConfig(t *testing.T) { - cfg := NewConfig() +// Move configuration init here to avoid race conditions when parsing flags in multiple tests +var cfg = NewConfig() + +func TestEnvOverrides_SwarmRedisPassword(t *testing.T) { + for _, tt := range []struct { + name string + args []string + env string + want string + }{ + { + name: "don't set redis password either from file nor environment", + args: []string{"skipper"}, + env: "", + want: "", + }, + { + name: "set redis password from environment", + args: []string{"skipper"}, + env: "set_from_env", + want: "set_from_env", + }, + { + name: "set redis password from config file and ignore environment", + args: []string{"skipper", "-config-file=test.yaml"}, + env: "set_from_env", + want: "set_from_file", + }, + } { + // Run test in non-concurrent way to avoid colisions of other test cases that parse config file + oldArgs := os.Args + defer func() { + os.Args = oldArgs + os.Unsetenv(redisPasswordEnv) + }() + os.Args = tt.args + if tt.env != "" { + os.Setenv(redisPasswordEnv, tt.env) + } + err := cfg.Parse() + if err != nil { + t.Errorf("config.NewConfig() error = %v", err) + } + if cfg.SwarmRedisPassword != tt.want { + t.Errorf("cfg.SwarmRedisPassword didn't set correctly: Want '%s', got '%s'", tt.want, cfg.SwarmRedisPassword) + } + } +} + +func Test_NewConfig(t *testing.T) { for _, tt := range []struct { name string args []string @@ -94,6 +142,7 @@ func Test_NewConfig(t *testing.T) { ResponseHeaderTimeoutBackend: 1 * time.Minute, ExpectContinueTimeoutBackend: 30 * time.Second, SwarmRedisURLs: commaListFlag(), + SwarmRedisPassword: "set_from_file", SwarmRedisDialTimeout: 25 * time.Millisecond, SwarmRedisReadTimeout: 25 * time.Millisecond, SwarmRedisWriteTimeout: 25 * time.Millisecond, diff --git a/config/test.yaml b/config/test.yaml index 4f8fd37d13..03e38d47fb 100644 --- a/config/test.yaml +++ b/config/test.yaml @@ -2,3 +2,4 @@ address: localhost:9090 max-loopbacks: 12 etcd-timeout: 2s status-checks: +swarm-redis-password: set_from_file diff --git a/ratelimit/redis.go b/ratelimit/redis.go index d869d6c665..9d2d63be80 100644 --- a/ratelimit/redis.go +++ b/ratelimit/redis.go @@ -18,6 +18,8 @@ import ( type RedisOptions struct { // Addrs are the list of redis shards Addrs []string + // Password is the password needed to connect to Redis server + Password string // DialTimeout is the max time.Duration to dial a new connection DialTimeout time.Duration // ReadTimeout for redis socket reads @@ -92,6 +94,7 @@ func newRing(ro *RedisOptions, quit <-chan struct{}) *ring { ringOptions.PoolTimeout = ro.PoolTimeout ringOptions.MinIdleConns = ro.MinIdleConns ringOptions.PoolSize = ro.MaxIdleConns + ringOptions.Password = ro.Password if ro.ConnMetricsInterval <= 0 { ro.ConnMetricsInterval = defaultConnMetricsInterval diff --git a/ratelimit/redis_test.go b/ratelimit/redis_test.go index 99d94667dc..f215b48504 100644 --- a/ratelimit/redis_test.go +++ b/ratelimit/redis_test.go @@ -1,30 +1,109 @@ -//+build redis - package ratelimit import ( "context" + "fmt" "log" "os/exec" "testing" "time" ) -func startRedis(port string) func() { +func startRedis(port, password string) func() { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - cmd := exec.CommandContext(ctx, "redis-server", "--port", port) + cmdArgs := []string{"--port", port} + if password != "" { + cmdArgs = append(cmdArgs, "--requirepass") + cmdArgs = append(cmdArgs, password) + } + cmd := exec.CommandContext(ctx, "redis-server", cmdArgs...) err := cmd.Start() if err != nil { log.Fatalf("Run '%q %q' failed, caused by: %s", cmd.Path, cmd.Args, err) } + return func() { cancel(); _ = cmd.Wait() } } +func Test_clusterLimitRedis_WithPass(t *testing.T) { + redisPort := "16379" + redisPassword := "pass" + + cancel := startRedis(redisPort, redisPassword) + defer cancel() + + clusterClientlimit := Settings{ + Type: ClusterClientRatelimit, + Lookuper: NewHeaderLookuper("X-Test"), + MaxHits: 5, + TimeWindow: time.Second, + Group: "Auth", + } + + tests := []struct { + name string + settings Settings + iterations int + args string + addrs []string + password string + want bool + }{ + { + name: "correct password", + settings: clusterClientlimit, + args: "clientAuth", + addrs: []string{fmt.Sprintf("127.0.0.1:%s", redisPort)}, + password: redisPassword, + iterations: 6, + want: false, + }, + { + name: "wrong password", + settings: clusterClientlimit, + args: "clientAuth", + addrs: []string{fmt.Sprintf("127.0.0.1:%s", redisPort)}, + password: "wrong", + iterations: 6, + want: true, + }, + { + name: "no password", + settings: clusterClientlimit, + args: "clientAuth", + addrs: []string{fmt.Sprintf("127.0.0.1:%s", redisPort)}, + password: "", + iterations: 6, + want: true, + }, + } + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + q := make(chan struct{}) + defer close(q) + c := newClusterRateLimiterRedis( + tt.settings, + newRing(&RedisOptions{Addrs: tt.addrs, Password: tt.password}, q), + tt.settings.Group, + ) + + var got bool + for i := 0; i < tt.iterations; i++ { + got = c.Allow(tt.args) + } + if got != tt.want { + t.Errorf("clusterLimitRedis.Allow() = %v, want %v", got, tt.want) + } + }) + } +} + func Test_clusterLimitRedis_Allow(t *testing.T) { redisPort := "16379" - cancel := startRedis(redisPort) + cancel := startRedis(redisPort, "") defer cancel() clusterlimit := Settings{ @@ -103,7 +182,7 @@ func Test_clusterLimitRedis_Allow(t *testing.T) { func Test_clusterLimitRedis_Delta(t *testing.T) { redisPort := "16380" - cancel := startRedis(redisPort) + cancel := startRedis(redisPort, "") defer cancel() clusterlimit := Settings{ @@ -169,7 +248,7 @@ func Test_clusterLimitRedis_Delta(t *testing.T) { func Test_clusterLimitRedis_Oldest(t *testing.T) { redisPort := "16381" - cancel := startRedis(redisPort) + cancel := startRedis(redisPort, "") defer cancel() clusterlimit := Settings{ @@ -236,7 +315,7 @@ func Test_clusterLimitRedis_Oldest(t *testing.T) { func Test_clusterLimitRedis_RetryAfter(t *testing.T) { redisPort := "16382" - cancel := startRedis(redisPort) + cancel := startRedis(redisPort, "") defer cancel() clusterlimit := Settings{ diff --git a/skipper.go b/skipper.go index 1ae07e24db..a904db5f6d 100644 --- a/skipper.go +++ b/skipper.go @@ -710,6 +710,7 @@ type Options struct { EnableSwarm bool // redis based swarm SwarmRedisURLs []string + SwarmRedisPassword string SwarmRedisDialTimeout time.Duration SwarmRedisReadTimeout time.Duration SwarmRedisWriteTimeout time.Duration @@ -1203,6 +1204,7 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error { log.Infof("Redis based swarm with %d shards", len(o.SwarmRedisURLs)) redisOptions = &ratelimit.RedisOptions{ Addrs: o.SwarmRedisURLs, + Password: o.SwarmRedisPassword, DialTimeout: o.SwarmRedisDialTimeout, ReadTimeout: o.SwarmRedisReadTimeout, WriteTimeout: o.SwarmRedisWriteTimeout,