fix(redis): address review feedback - warn on ignored credentials, fix feature gating

Author: ex0a

Cargo.toml

@@ -125,7 +125,7 @@ rouille = { version = "3.6", optional = true, default-features = false, features
 syslog = { version = "7", optional = true }
 version-compare = { version = "0.1.1", optional = true }
 tracing = "0.1.44"
-redis = { version = "0.32", features = ["sentinel"] }
+redis = { version = "0.32", features = ["sentinel"], optional = true }
 
 [dev-dependencies]
 assert_cmd = "2.0.13"
@@ -159,7 +159,7 @@ version = "0.52"
 [features]
 all = [
   "dist-client",
-
+  "redis",
   "s3",
   "memcached",
   "gcs",
@@ -177,7 +177,7 @@ gha = ["opendal/services-ghac", "reqwest"]
 memcached = ["opendal/services-memcached"]
 native-zlib = []
 oss = ["opendal/services-oss", "reqsign", "reqwest"]
-redis = ["url", "opendal/services-redis"]
+redis = ["dep:redis", "url", "opendal/services-redis"]
 s3 = ["opendal/services-s3", "reqsign", "reqwest"]
 webdav = ["opendal/services-webdav", "reqwest"]
 # Enable features that will build a vendored version of openssl and

src/cache/cache.rs

@@ -380,6 +380,9 @@ pub fn build_single_cache(
                 (Some(url_str), None, None) => {
                     if url_str.starts_with("redis-sentinel://") {
                         debug!("Init redis sentinel cache with url {url_str}");
+                        if username.is_some() || password.is_some() || *db != crate::config::DEFAULT_REDIS_DB {
+                            warn!("`username`, `password` and `db` have no effect when using a `redis-sentinel://` URL. Embed credentials in the URL instead.");
+                        }
                         RedisCache::build_from_url(url_str, key_prefix, *ttl)
                     } else {
                         debug!("Init redis single-node cache with url {url_str}");

src/cache/redis.rs

@@ -21,17 +21,13 @@ use std::collections::HashMap;
 use std::time::Duration;
 use url::Url;
 use anyhow::anyhow;
-use tracing::info;
 
 /// A cache that stores entries in a Redis.
 pub struct RedisCache;
 
 impl RedisCache {
     /// Create a new `RedisCache` for the given URL.
     pub fn build_from_url(url: &str, key_prefix: &str, ttl: u64) -> Result<Operator> {
-        if url.starts_with("redis-sentinel://") {
-            return Self::build_sentinel(url, key_prefix, ttl);
-        }
         let parsed = Url::parse(url)?;
 
         let mut builder = Redis::default()
@@ -62,7 +58,7 @@ impl RedisCache {
     pub fn build_sentinel(url: &str, key_prefix: &str, ttl: u64) -> Result<Operator> {
         use std::net::ToSocketAddrs;
 
-        info!("Building Redis Sentinel cache from URL: {}", url);
+        debug!("Building Redis Sentinel cache from URL: {}", url);
 
         // Basic parsing for: redis-sentinel://[:password@]host1[:port1][,host2[:port2],...]/master_name[/db]
         let clean_url = url.trim_start_matches("redis-sentinel://");
@@ -74,7 +70,7 @@ impl RedisCache {
         let nodes_part = parts[0];
         let master_name = parts[1];
 
-        info!("Sentinel nodes: {}, master_name: {}", nodes_part, master_name);
+        debug!("Sentinel nodes: {}, master_name: {}", nodes_part, master_name);
 
         // Handle password if present
         let (password, nodes_str) = if nodes_part.contains('@') {
@@ -89,30 +85,30 @@ impl RedisCache {
         let mut master_addr = None;
         let mut last_error: Option<String> = None;
 
-        info!("Attempting to discover master '{}' from {} sentinel node(s)", master_name, nodes_raw.len());
+        debug!("Attempting to discover master '{}' from {} sentinel node(s)", master_name, nodes_raw.len());
 
         for node in &nodes_raw {
-            info!("Trying sentinel node: {}", node);
+            debug!("Trying sentinel node: {}", node);
 
             // Resolve hostname to IP address(es)
             let resolved_addr = match node.to_socket_addrs() {
                 Ok(mut addrs) => {
                     if let Some(addr) = addrs.next() {
-                        info!("Resolved {} to {}", node, addr);
+                        debug!("Resolved {} to {}", node, addr);
                         addr.to_string()
                     } else {
-                        info!("DNS resolved {} but returned no addresses", node);
+                        debug!("DNS resolved {} but returned no addresses", node);
                         node.to_string()
                     }
                 }
                 Err(e) => {
-                    info!("DNS resolution failed for {}: {}, using hostname directly", node, e);
+                    debug!("DNS resolution failed for {}: {}, using hostname directly", node, e);
                     node.to_string()
                 }
             };
 
             let redis_url = format!("redis://{}", resolved_addr);
-            info!("Connecting to sentinel at: {}", redis_url);
+            debug!("Connecting to sentinel at: {}", redis_url);
 
             match redis::Client::open(redis_url.as_str()) {
                 Ok(client) => {
@@ -126,29 +122,29 @@ impl RedisCache {
                             match res {
                                 Ok(addr_parts) if addr_parts.len() >= 2 => {
                                     let discovered = format!("redis://{}:{}", addr_parts[0], addr_parts[1]);
-                                    info!("Discovered master '{}' at: {}", master_name, discovered);
+                                    debug!("Discovered master '{}' at: {}", master_name, discovered);
                                     master_addr = Some(discovered);
                                     break;
                                 }
                                 Ok(addr_parts) => {
                                     last_error = Some(format!("Sentinel returned incomplete response: {:?}", addr_parts));
-                                    info!("{}", last_error.as_ref().unwrap());
+                                    debug!("{}", last_error.as_ref().unwrap());
                                 }
                                 Err(e) => {
                                     last_error = Some(format!("Sentinel query failed: {}", e));
-                                    info!("{}", last_error.as_ref().unwrap());
+                                    debug!("{}", last_error.as_ref().unwrap());
                                 }
                             }
                         }
                         Err(e) => {
                             last_error = Some(format!("Connection failed: {}", e));
-                            info!("{}", last_error.as_ref().unwrap());
+                            debug!("{}", last_error.as_ref().unwrap());
                         }
                     }
                 }
                 Err(e) => {
                     last_error = Some(format!("Client creation failed: {}", e));
-                    info!("{}", last_error.as_ref().unwrap());
+                    debug!("{}", last_error.as_ref().unwrap());
                 }
             }
         }
@@ -161,7 +157,7 @@ impl RedisCache {
             }
         };
 
-        info!("Using Redis master endpoint: {}", final_endpoint);
+        debug!("Using Redis master endpoint: {}", final_endpoint);
 
         let mut builder = Redis::default()
             .endpoint(&final_endpoint)
@@ -186,7 +182,7 @@ impl RedisCache {
             .layer(LoggingLayer::default())
             .finish();
 
-        info!("Redis Sentinel cache initialized successfully");
+        debug!("Redis Sentinel cache initialized successfully");
         Ok(op)
     }