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 <[email protected]> * Set Redis password from config file Signed-off-by: Konstantinos Christofilos <[email protected]>
zalando · Feb 9, 2021 · 4a4eb65 · 4a4eb65
1 parent 6ab914e
commit 4a4eb65
Show file tree

Hide file tree

Showing 6 changed files with 159 additions and 11 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,