Password-protected Redis storage provider connection errors

[cadams93]

Component(s)

router

router version

v0.170.0

What happened?

Description

The recent PR #1499 has introduced the ability to connect to Redis, when running in cluster-mode, which was previously unsupported. As a result of this change, it is my understanding that connecting to Redis clusters or indeed standalone instances with a password is no longer functioning correctly.

https://github.com/wundergraph/cosmo/pull/1499/files#diff-3173118cd9e8f3849a4d398d232609e32cac4991fd962e78280d9c68c14b67abR26 - NewRedisCloser is passed an options bundle, which never has Password populated (hence will be "").

• For cluster_mode: false, the password parsed from the URL using the go-redis library is replaced by the value provided in this options struct.
• For cluster_mode: true, the password is mapped into the go-redis library's ClusterOptions struct

In both cases, having an "" value will cause issues for connections requiring authentication, and result in a NOAUTH response from the Redis server.

Additionally TLS connections using the rediss:// URL scheme appear to be unsupported when using cluster_enabled: true. The manual stripping of the scheme from each URL strips only redis:// (non-TLS) URLs. Doing this outside of the go-redis library also neglects to configure the ClusterOptions.TLSConfig option, which results in a non-TLS connection being attempted to a TLS Redis instance, also failing.

fwiw: a symmetrical function to redis.ParseURL exists in the backing go-redis library, specifically for parsing cluster-mode URLs into the ClusterOptions struct, called redis.ParseClusterURL. If used, this would provide greater flexibility for consumers, allowing timeouts, connection pools, etc to be configured.

Steps to Reproduce

• Scenario 1: Standalone Redis connections *

1. Start a local Redis instance: docker run -p 6379:6379 redis
2. Set a password on the instance, by connecting with redis-cli and running:

CONFIG SET requirepass "mypass"

3. Configure Cosmo Router with a Redis storage provider:

#... 
 storage:
    cluster_enabled: false
    urls:
      - "redis://default:mypass@localhost:6379"
# ...

4. Start Cosmo Router
5. Notice error in logs:

failed to bootstrap router: failed to create a functioning redis client 

• Scenario 2: Cluster mode Redis connections *

1. Start a local Redis cluster-mode cluster, with a password of "mypass"
2. Configure Cosmo Router with a Redis storage provider:

#... 
 storage:
    cluster_enabled: true
    urls:
      - "redis://default:mypass@localhost:6379"
      - "redis://default:mypass@localhost:6380"
      - "redis://default:mypass@localhost:6381"
# ...

3. Start Cosmo Router
4. Notice similar error in logs

(For the TLS issue, supply rediss:// as the scheme, and notice this is not stripped in the URLs supplied to the underlying library)

Expected Result

Connection to Redis cluster-mode and/or standalone instances over non-TLS and/or TLS connections, with the ability to supply a valid password.

Actual Result

• failed to bootstrap router: failed to create a functioning redis client

Log output

failed to bootstrap router: failed to create a functioning redis client 

[github-actions]

WunderGraph commits fully to Open Source and we want to make sure that we can help you as fast as possible.
The roadmap is driven by our customers and we have to prioritize issues that are important to them.
You can influence the priority by becoming a customer. Please contact us here.

[cadams93]

As mentioned in the original issue, this appears to be a regression for standalone password-protected Redis connections, as running v0.169.0 of router with equivalent config does not result in the same error and successfully connects:

#... 
 storage:
    url: "redis://default:mypass@localhost:6379"
# ...

[df-wg]

@cadams93 Thank you for the extremely clear issue write up! I'm going to get a fix for this out ASAP

[df-wg]

Hey @cadams93 , we just released a fix in [email protected]. Let me know if that works for you!

[iDub79]

Appreciate the quick turnaround @df-wg . I have tested the latest changes and it is now working for us.