fix(router): enable redis to work with auth (#1563)

wundergraph · Feb 5, 2025 · d9fd035 · d9fd035
1 parent 5f1d2a7
commit d9fd035
Show file tree

Hide file tree

Showing 10 changed files with 248 additions and 48 deletions.
diff --git a/.github/workflows/router-ci.yaml b/.github/workflows/router-ci.yaml
@@ -159,13 +159,23 @@ jobs:
       - name: Setup Redis Cluster (for Cluster tests)
         uses: vishnudxb/redis-cluster@1.0.9
         with:
-          master1-port: 7000
-          master2-port: 7001
-          master3-port: 7002
-          slave1-port: 7003
-          slave2-port: 7004
-          slave3-port: 7005
+          master1-port: 7001
+          master2-port: 7002
+          master3-port: 7003
+          slave1-port: 7004
+          slave2-port: 7005
+          slave3-port: 7006
           sleep-duration: 5
+      - name: Configure Redis Authentication & ACL
+        run: |
+          sudo apt-get install -y redis-tools
+          docker ps -a
+          # Set a password for each master node
+          for port in 7001 7002 7003; do
+            redis-cli -h 127.0.0.1 -p $port ACL SETUSER cosmo on ">test" "~*" "+@all"
+            redis-cli -u "redis://cosmo:test@127.0.0.1:$port" ping
+            echo "ACL user 'cosmo' created with full access on port $port"
+          done
       - uses: nick-fields/retry@v3
         with:
           timeout_minutes: 30

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -243,38 +243,41 @@ services:
     profiles:
       - dev
 
-#  3 node minimum for a cluster, per redis documentation
-  redis-cluster-node-0:
+  #  3 node minimum for a cluster, per redis documentation
+  redis-cluster-node-1:
     image: redis:${DC_REDIS_VERSION:-7.2.4}-alpine
     command: redis-server /usr/local/etc/redis/redis.conf
     networks:
       - primary
     ports:
-      - "7000:6379"
+      - "7001:6379"
+      - "16371:16379"
     volumes:
       - ./docker/redis/redis-cluster.conf:/usr/local/etc/redis/redis.conf
     profiles:
       - dev
 
-  redis-cluster-node-1:
+  redis-cluster-node-2:
     image: redis:${DC_REDIS_VERSION:-7.2.4}-alpine
     command: redis-server /usr/local/etc/redis/redis.conf
     networks:
       - primary
     ports:
-      - "7001:6379"
+      - "7002:6379"
+      - "16372:16379"
     volumes:
       - ./docker/redis/redis-cluster.conf:/usr/local/etc/redis/redis.conf
     profiles:
       - dev
 
-  redis-cluster-node-2:
+  redis-cluster-node-3:
     image: redis:${DC_REDIS_VERSION:-7.2.4}-alpine
     command: redis-server /usr/local/etc/redis/redis.conf
     networks:
       - primary
     ports:
-      - "7002:6379"
+      - "7003:6379"
+      - "16373:16379"
     volumes:
       - ./docker/redis/redis-cluster.conf:/usr/local/etc/redis/redis.conf
     profiles:
@@ -286,9 +289,9 @@ services:
     networks:
       - primary
     depends_on:
-      - redis-cluster-node-0
       - redis-cluster-node-1
       - redis-cluster-node-2
+      - redis-cluster-node-3
     volumes:
       - ./docker/redis/:/usr/local/etc/redis/
     profiles:
@@ -331,6 +334,6 @@ volumes:
   redis:
   redis-slave:
   redis-cluster-configure:
-  redis-cluster-node-0:
   redis-cluster-node-1:
   redis-cluster-node-2:
+  redis-cluster-node-3:
diff --git a/docker/redis/redis-cluster-create.sh b/docker/redis/redis-cluster-create.sh
@@ -1,13 +1,21 @@
 # wait for the docker-compose depends_on to spin up the redis nodes usually takes this long
 sleep 10
 
-node_0_ip=$(getent hosts redis-cluster-node-0 | awk '{ print $1 }')
 node_1_ip=$(getent hosts redis-cluster-node-1 | awk '{ print $1 }')
 node_2_ip=$(getent hosts redis-cluster-node-2 | awk '{ print $1 }')
+node_3_ip=$(getent hosts redis-cluster-node-3 | awk '{ print $1 }')
 
+# Set cluster properties dynamically for each node
+for ip in $node_1_ip $node_2_ip $node_3_ip; do
+  echo "Configuring Redis node at $ip"
+  redis-cli -h $ip -p 6379 CONFIG SET cluster-announce-ip "$ip"
+done
 
+# Create the cluster
 redis-cli --cluster create \
-  $node_0_ip:6379 \
   $node_1_ip:6379 \
   $node_2_ip:6379 \
-  --cluster-replicas 0 --cluster-yes
+  $node_3_ip:6379 \
+  --cluster-replicas 0 --cluster-yes
+
+echo "Redis Cluster setup complete!"
diff --git a/docker/redis/redis-cluster.conf b/docker/redis/redis-cluster.conf
@@ -6,4 +6,7 @@ protected-mode no
 maxmemory 100mb
 maxmemory-policy noeviction
 save 60 1
-appendonly no
+appendonly no
+cluster-announce-port 6379
+cluster-announce-bus-port 16379
+user cosmo on >test ~* &* +@all
diff --git a/router-tests/automatic_persisted_queries_test.go b/router-tests/automatic_persisted_queries_test.go
@@ -163,6 +163,7 @@ func TestAutomaticPersistedQueries(t *testing.T) {
 		var (
 			redisLocalUrl = "localhost:6379"
 			redisUrl      = fmt.Sprintf("redis://%s", redisLocalUrl)
+			redisUser     = "cosmo"
 			redisPassword = "test"
 			client        = redis.NewClient(&redis.Options{Addr: redisLocalUrl, Password: redisPassword})
 		)
@@ -366,10 +367,11 @@ func TestAutomaticPersistedQueries(t *testing.T) {
 
 		t.Run("works with cluster mode", func(t *testing.T) {
 			t.Parallel()
-
-			clusterUrls := []string{"localhost:7000", "localhost:7001"}
+			clusterUrls := []string{"redis://cosmo:test@localhost:7002", "redis://cosmo:test@localhost:7001"}
+			noSchemeClusterUrls := []string{"localhost:7002", "localhost:7001"}
 			clusterClient := redis.NewClusterClient(&redis.ClusterOptions{
-				Addrs:    clusterUrls,
+				Addrs:    noSchemeClusterUrls,
+				Username: redisUser,
 				Password: redisPassword,
 			})
 

diff --git a/router-tests/ratelimit_test.go b/router-tests/ratelimit_test.go
@@ -675,16 +675,112 @@ func TestRateLimit(t *testing.T) {
 	})
 	t.Run("Cluster Mode", func(t *testing.T) {
 		var (
-			clusterUrlSlice = []string{"localhost:7000", "localhost:7001", "localhost:7002"}
-			password        = "test"
+			clusterUrlSlice     = []string{"redis://cosmo:test@localhost:7001", "redis://cosmo:test@localhost:7002", "redis://cosmo:test@localhost:7003"}
+			noSchemeClusterUrls = []string{"localhost:7001", "localhost:7002", "localhost:7003"}
+			user                = "cosmo"
+			password            = "test"
 		)
 
+		t.Run("correctly parses url options and authentication", func(t *testing.T) {
+			t.Parallel()
+
+			tests := []struct {
+				name            string
+				clusterUrlSlice []string
+			}{
+				{
+					name:            "should successfully use auth from first url",
+					clusterUrlSlice: []string{"redis://cosmo:test@localhost:7003", "redis://cosmo1:test1@localhost:7001", "redis://cosmo2:test2@localhost:7002"},
+				},
+				{
+					name:            "should successfully use auth from later url if no auth in first urls",
+					clusterUrlSlice: []string{"redis://localhost:7003", "rediss://localhost:7001", "rediss://cosmo:test@localhost:7002"},
+				},
+				{
+					name:            "should successfully work with two urls",
+					clusterUrlSlice: []string{"redis://cosmo:test@localhost:7002", "rediss://localhost:7001"},
+				},
+			}
+
+			for _, tt := range tests {
+				t.Run(tt.name, func(t *testing.T) {
+					t.Parallel()
+					key := uuid.New().String()
+					t.Cleanup(func() {
+						client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: noSchemeClusterUrls, Username: user, Password: password})
+						del := client.Del(context.Background(), key)
+						require.NoError(t, del.Err())
+					})
+
+					testenv.Run(t, &testenv.Config{
+						RouterOptions: []core.Option{
+							core.WithRateLimitConfig(&config.RateLimitConfiguration{
+								Enabled:  true,
+								Strategy: "simple",
+								SimpleStrategy: config.RateLimitSimpleStrategy{
+									Rate:                    1,
+									Burst:                   1,
+									Period:                  time.Second * 2,
+									RejectExceedingRequests: false,
+								},
+								Storage: config.RedisConfiguration{
+									ClusterEnabled: true,
+									URLs:           tt.clusterUrlSlice,
+									KeyPrefix:      key,
+								},
+								Debug: true,
+							}),
+						},
+					}, func(t *testing.T, xEnv *testenv.Environment) {
+						res := xEnv.MakeGraphQLRequestOK(testenv.GraphQLRequest{
+							Query:     `query ($n:Int!) { employee(id:$n) { id details { forename surname } } }`,
+							Variables: json.RawMessage(`{"n":1}`),
+						})
+						require.Equal(t, fmt.Sprintf(`{"data":{"employee":{"id":1,"details":{"forename":"Jens","surname":"Neuse"}}},"extensions":{"rateLimit":{"key":"%s","requestRate":1,"remaining":0,"retryAfterMs":1234,"resetAfterMs":1234}}}`, key), res.Body)
+					})
+				})
+			}
+
+			t.Run("should fail with bad auth", func(t *testing.T) {
+				t.Parallel()
+				clusterUrlSlice = []string{"redis://cosmo1:test1@localhost:7001", "redis://cosmo:test@localhost:7003", "redis://cosmo2:test2@localhost:7002"}
+
+				key := uuid.New().String()
+				t.Cleanup(func() {
+					client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: noSchemeClusterUrls, Username: user, Password: password})
+					del := client.Del(context.Background(), key)
+					require.NoError(t, del.Err())
+				})
+				testenv.FailsOnStartup(t, &testenv.Config{
+					RouterOptions: []core.Option{
+						core.WithRateLimitConfig(&config.RateLimitConfiguration{
+							Enabled:  true,
+							Strategy: "simple",
+							SimpleStrategy: config.RateLimitSimpleStrategy{
+								Rate:                    1,
+								Burst:                   1,
+								Period:                  time.Second * 2,
+								RejectExceedingRequests: false,
+							},
+							Storage: config.RedisConfiguration{
+								ClusterEnabled: true,
+								URLs:           clusterUrlSlice,
+								KeyPrefix:      key,
+							},
+							Debug: true,
+						}),
+					},
+				}, func(t *testing.T, err error) {
+					require.Contains(t, err.Error(), "failed to create a functioning redis client")
+				})
+			})
+		})
 		t.Run("enabled - below limit", func(t *testing.T) {
 			t.Parallel()
 
 			key := uuid.New().String()
 			t.Cleanup(func() {
-				client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: clusterUrlSlice, Password: password})
+				client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: noSchemeClusterUrls, Username: user, Password: password})
 				del := client.Del(context.Background(), key)
 				require.NoError(t, del.Err())
 			})
@@ -720,7 +816,7 @@ func TestRateLimit(t *testing.T) {
 
 			key := uuid.New().String()
 			t.Cleanup(func() {
-				client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: clusterUrlSlice, Password: password})
+				client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: noSchemeClusterUrls, Username: user, Password: password})
 				del := client.Del(context.Background(), fmt.Sprintf("%s:localhost", key))
 				require.NoError(t, del.Err())
 			})
@@ -760,7 +856,7 @@ func TestRateLimit(t *testing.T) {
 
 			key := uuid.New().String()
 			t.Cleanup(func() {
-				client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: clusterUrlSlice, Password: password})
+				client := redis.NewClusterClient(&redis.ClusterOptions{Addrs: noSchemeClusterUrls, Username: user, Password: password})
 				del := client.Del(context.Background(), key)
 				require.NoError(t, del.Err())
 			})

diff --git a/router-tests/testenv/testenv.go b/router-tests/testenv/testenv.go
@@ -106,6 +106,15 @@ func Run(t *testing.T, cfg *Config, f func(t *testing.T, xEnv *Environment)) {
 	}
 }
 
+// FailsOnStartup runs the test and ensures that the router fails during bootstrapping
+func FailsOnStartup(t *testing.T, cfg *Config, f func(t *testing.T, err error)) {
+	t.Helper()
+	env, err := createTestEnv(t, cfg)
+	require.Error(t, err)
+	require.Nil(t, env)
+	f(t, err)
+}
+
 // RunWithError runs the test but returns an error instead of failing the test
 // Useful when you want to assert errors during router bootstrapping
 func RunWithError(t *testing.T, cfg *Config, f func(t *testing.T, xEnv *Environment)) error {