From 374054d6e1ef34dbbd0486c647a1dd764423dca4 Mon Sep 17 00:00:00 2001 From: Benjamin <5719034+bnjjj@users.noreply.github.com> Date: Thu, 27 Nov 2025 16:12:19 +0100 Subject: [PATCH 1/3] fix(response_cache): don't require redis when disabled and require it when enabled Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com> --- .../src/plugins/response_cache/plugin.rs | 382 +++++++++++++++--- .../src/plugins/response_cache/tests.rs | 206 +++++++--- 2 files changed, 471 insertions(+), 117 deletions(-) diff --git a/apollo-router/src/plugins/response_cache/plugin.rs b/apollo-router/src/plugins/response_cache/plugin.rs index d25800717a..84c8615ecb 100644 --- a/apollo-router/src/plugins/response_cache/plugin.rs +++ b/apollo-router/src/plugins/response_cache/plugin.rs @@ -297,19 +297,35 @@ impl PluginPrivate for ResponseCache { let mut storage_interface = StorageInterface::default(); let (drop_tx, drop_rx) = tokio::sync::broadcast::channel(2); - if let Some(config) = init.config.subgraph.all.redis.clone() { + if init.config.enabled + && init.config.subgraph.all.enabled.unwrap_or_default() + && let Some(config) = init.config.subgraph.all.redis.clone() + { let storage = Arc::new(OnceLock::new()); storage_interface.all = Some(storage.clone()); connect_or_spawn_reconnection_task(config, storage, drop_rx).await?; } for (subgraph, subgraph_config) in &init.config.subgraph.subgraphs { - if let Some(config) = subgraph_config.redis.clone() { - let storage = Arc::new(OnceLock::new()); - storage_interface - .subgraphs - .insert(subgraph.clone(), storage.clone()); - connect_or_spawn_reconnection_task(config, storage, drop_tx.subscribe()).await?; + if Self::static_subgraph_enabled(init.config.enabled, &init.config.subgraph, subgraph) { + match subgraph_config.redis.clone() { + Some(config) => { + let storage = Arc::new(OnceLock::new()); + storage_interface + .subgraphs + .insert(subgraph.clone(), storage.clone()); + connect_or_spawn_reconnection_task(config, storage, drop_tx.subscribe()) + .await?; + } + None => { + if storage_interface.all.is_none() { + return Err( + format!("you must have a redis configured either for all subgraphs or for subgraph {subgraph:?}") + .into(), + ); + } + } + } } } @@ -497,7 +513,7 @@ impl ResponseCache { ))] pub(crate) async fn for_test( storage: Storage, - subgraphs: HashMap, + subgraphs: SubgraphConfiguration, supergraph_schema: Arc>, truncate_namespace: bool, drop_tx: broadcast::Sender<()>, @@ -522,16 +538,7 @@ impl ResponseCache { entity_type: None, enabled: true, debug: true, - subgraphs: Arc::new(SubgraphConfiguration { - all: Subgraph { - invalidation: Some(SubgraphInvalidationConfig { - enabled: true, - shared_key: INVALIDATION_SHARED_KEY.to_string(), - }), - ..Default::default() - }, - subgraphs, - }), + subgraphs: Arc::new(subgraphs), private_queries: Arc::new(RwLock::new(LruCache::new(DEFAULT_LRU_PRIVATE_QUERIES_SIZE))), endpoint_config: Some(Arc::new(InvalidationEndpointConfig { path: String::from("/invalidation"), @@ -601,14 +608,23 @@ impl ResponseCache { }) } - // Returns boolean to know if cache is enabled for this subgraph + /// Returns boolean to know if cache is enabled for this subgraph fn subgraph_enabled(&self, subgraph_name: &str) -> bool { - if !self.enabled { + Self::static_subgraph_enabled(self.enabled, &self.subgraphs, subgraph_name) + } + + /// Static method which returns boolean to know if cache is enabled for this subgraph + fn static_subgraph_enabled( + plugin_enabled: bool, + subgraph_config: &SubgraphConfiguration, + subgraph_name: &str, + ) -> bool { + if !plugin_enabled { return false; } match ( - self.subgraphs.all.enabled, - self.subgraphs.get(subgraph_name).enabled, + subgraph_config.all.enabled, + subgraph_config.get(subgraph_name).enabled, ) { (_, Some(x)) => x, // explicit per-subgraph setting overrides the `all` default (Some(true) | None, None) => true, // unset defaults to true @@ -2653,6 +2669,7 @@ mod tests { use super::Subgraph; use super::Ttl; use crate::configuration::subgraph::SubgraphConfiguration; + use crate::plugin::PluginInit; use crate::plugin::PluginPrivate; use crate::plugins::response_cache::plugin::ResponseCache; use crate::plugins::response_cache::plugin::get_entity_key_from_selection_set; @@ -2661,6 +2678,7 @@ mod tests { use crate::plugins::response_cache::plugin::matches_selection_set; use crate::plugins::response_cache::storage::redis::Config; use crate::plugins::response_cache::storage::redis::Storage; + use crate::plugins::response_cache::tests::create_subgraph_conf; use crate::services::OperationKind; use crate::services::subgraph; @@ -2673,7 +2691,7 @@ mod tests { let storage = Storage::new(&Config::test(false, "test_subgraph_enabled"), drop_rx) .await .unwrap(); - let map = serde_json_bytes::json!({ + let map = serde_json_bytes::from_value(serde_json_bytes::json!({ "user": { "private_id": "sub" }, @@ -2685,11 +2703,13 @@ mod tests { "private_id": "sub", "enabled": false } - }); + })) + .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); let mut response_cache = ResponseCache::for_test( storage.clone(), - serde_json_bytes::from_value(map).unwrap(), + subgraphs_conf, valid_schema.clone(), true, drop_tx, @@ -2722,29 +2742,9 @@ mod tests { let storage = Storage::new(&Config::test(false, &Uuid::new_v4().to_string()), drop_rx) .await .unwrap(); - let map = serde_json_bytes::json!({ - "user": { - "private_id": "sub" - }, - "orga": { - "private_id": "sub", - "enabled": true - }, - "archive": { - "private_id": "sub", - "enabled": false - } - }); - let mut response_cache = ResponseCache::for_test( + + ResponseCache::for_test( storage.clone(), - serde_json_bytes::from_value(map).unwrap(), - valid_schema.clone(), - true, - drop_tx, - ) - .await - .unwrap(); - response_cache.subgraphs = Arc::new( serde_json_bytes::from_value(serde_json_bytes::json!({ "all": { "enabled": all_enabled, @@ -2765,9 +2765,288 @@ mod tests { } })) .unwrap(), + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap() + } + + #[tokio::test] + async fn test_redis_connection_disabled() { + let valid_schema = Arc::new(Schema::parse_and_validate(SCHEMA, "test.graphql").unwrap()); + let config: super::Config = serde_json_bytes::from_value(serde_json_bytes::json!({ + "enabled": true, + "subgraph": { + "all": { + "enabled": false, + "ttl": "10s", + "redis": { + "urls": ["redis://127.0.0.1:6379"], + "namespace": Uuid::new_v4().to_string(), + "pool_size": 1, + "required_to_start": true, + } + }, + "subgraphs": { + "user": { + "enabled": false + } + } + } + })) + .unwrap(); + let response_cache = ResponseCache::new(PluginInit::fake_new( + config, + Arc::new(valid_schema.to_string()), + )) + .await + .unwrap(); + + assert!( + response_cache.storage.all.is_none() + || response_cache.storage.all.as_ref().unwrap().get().is_none(), + "Redis storage is set globally" + ); + assert!( + response_cache.storage.subgraphs.is_empty(), + "Redis storage is set for a subgraph" ); + // ----- Disable globally the plugin ---- + let config: super::Config = serde_json_bytes::from_value(serde_json_bytes::json!({ + "enabled": false, + "subgraph": { + "all": { + "enabled": true, + "ttl": "10s", + "redis": { + "urls": ["redis://127.0.0.1:6379"], + "namespace": Uuid::new_v4().to_string(), + "pool_size": 1, + "required_to_start": true, + } + }, + "subgraphs": { + "user": { + "enabled": true + } + } + } + })) + .unwrap(); + let response_cache = ResponseCache::new(PluginInit::fake_new( + config, + Arc::new(valid_schema.to_string()), + )) + .await + .unwrap(); - response_cache + assert!( + response_cache.storage.all.is_none() + || response_cache.storage.all.as_ref().unwrap().get().is_none(), + "Redis storage is set globally" + ); + assert!( + response_cache.storage.subgraphs.is_empty(), + "Redis storage is set for a subgraph" + ); + } + + #[tokio::test] + async fn test_no_redis_conf_provided_should_fail() { + let valid_schema = Arc::new(Schema::parse_and_validate(SCHEMA, "test.graphql").unwrap()); + let config: super::Config = serde_json_bytes::from_value(serde_json_bytes::json!({ + "enabled": true, + "subgraph": { + "all": { + "enabled": true, + "ttl": "10s", + }, + "subgraphs": { + "user": { + "enabled": true + }, + "inventory": { + "enabled": true + } + } + } + })) + .unwrap(); + assert!( + ResponseCache::new(PluginInit::fake_new( + config, + Arc::new(valid_schema.to_string()), + )) + .await + .is_err(), + "The plugin should not start properly if caching is enabled but no redis provided" + ); + } + + #[tokio::test] + #[rstest::rstest] + // Globally disabled + #[case(false, true, true)] + // Disable for all subgraphs + #[case(true, false, false)] + async fn test_no_redis_conf_provided_but_disabled_should_succeed( + #[case] global_enabled: bool, + #[case] all_enabled: bool, + #[case] subgraph_enabled: bool, + ) { + let valid_schema = Arc::new(Schema::parse_and_validate(SCHEMA, "test.graphql").unwrap()); + let config: super::Config = serde_json_bytes::from_value(serde_json_bytes::json!({ + "enabled": global_enabled, + "subgraph": { + "all": { + "enabled": all_enabled, + "ttl": "10s", + }, + "subgraphs": { + "user": { + "enabled": subgraph_enabled + }, + "inventory": { + "enabled": subgraph_enabled + } + } + } + })) + .unwrap(); + let response_cache = ResponseCache::new(PluginInit::fake_new( + config, + Arc::new(valid_schema.to_string()), + )) + .await + .unwrap(); + if !global_enabled { + assert!(!response_cache.enabled); + } + assert!( + response_cache.storage.all.is_none() + || response_cache.storage.all.as_ref().unwrap().get().is_none(), + "Redis storage is set globally" + ); + assert!( + response_cache.storage.subgraphs.is_empty(), + "Redis storage is set for a subgraph" + ); + } + + #[tokio::test] + async fn test_redis_connection_enabled_multiple_subgraphs() { + let valid_schema = Arc::new(Schema::parse_and_validate(SCHEMA, "test.graphql").unwrap()); + let config: super::Config = serde_json_bytes::from_value(serde_json_bytes::json!({ + "enabled": true, + "subgraph": { + "all": { + "enabled": false, + "ttl": "10s", + "redis": { + "urls": ["redis://127.0.0.1:6379"], + "namespace": Uuid::new_v4().to_string(), + "pool_size": 1, + "required_to_start": true, + } + }, + "subgraphs": { + "user": { + "enabled": false + }, + "inventory": { + "enabled": true + } + } + } + })) + .unwrap(); + let response_cache = ResponseCache::new(PluginInit::fake_new( + config, + Arc::new(valid_schema.to_string()), + )) + .await + .unwrap(); + + assert!( + response_cache.storage.all.is_none() + || response_cache.storage.all.as_ref().unwrap().get().is_none(), + "Redis storage is set globally" + ); + assert_eq!( + response_cache.storage.subgraphs.len(), + 1, + "Redis storage is not set for a subgraph" + ); + } + + #[tokio::test] + #[rstest::rstest] + // Everything enabled + #[case(true, true)] + // Enable caching only for a specific subgraph should enable redis + #[case(false, true)] + // Enable caching for all subgraphs should enable redis + #[case(true, false)] + async fn test_redis_connection_enabled( + #[case] all_enabled: bool, + #[case] subgraph_enabled: bool, + ) { + let valid_schema = Arc::new(Schema::parse_and_validate(SCHEMA, "test.graphql").unwrap()); + let config: super::Config = serde_json_bytes::from_value(serde_json_bytes::json!({ + "enabled": true, + "subgraph": { + "all": { + "enabled": all_enabled, + "ttl": "10s", + "redis": { + "urls": ["redis://127.0.0.1:6379"], + "namespace": Uuid::new_v4().to_string(), + "pool_size": 1, + "required_to_start": true, + } + }, + "subgraphs": { + "user": { + "enabled": subgraph_enabled + } + } + } + })) + .unwrap(); + let response_cache = ResponseCache::new(PluginInit::fake_new( + config, + Arc::new(valid_schema.to_string()), + )) + .await + .unwrap(); + + if all_enabled { + assert!( + response_cache.storage.all.is_some() + && response_cache.storage.all.as_ref().unwrap().get().is_some(), + "Redis storage is not set globally" + ); + } else { + assert!( + response_cache.storage.all.is_none() + || response_cache.storage.all.as_ref().unwrap().get().is_none(), + "Redis storage is set globally" + ); + } + if subgraph_enabled { + assert_eq!( + response_cache.storage.subgraphs.len(), + 1, + "Redis storage is set for a subgraph" + ); + } else { + assert!( + response_cache.storage.subgraphs.is_empty(), + "Redis storage is not set for a subgraph" + ); + } } #[tokio::test] @@ -2873,7 +3152,7 @@ mod tests { let storage = Storage::new(&Config::test(false, "test_subgraph_ttl"), drop_rx) .await .unwrap(); - let map = serde_json_bytes::json!({ + let map = serde_json_bytes::from_value(serde_json_bytes::json!({ "user": { "private_id": "sub", "ttl": "2s" @@ -2887,11 +3166,12 @@ mod tests { "enabled": false, "ttl": "5000ms" } - }); + })) + .unwrap(); let mut response_cache = ResponseCache::for_test( storage.clone(), - serde_json_bytes::from_value(map).unwrap(), + create_subgraph_conf(map), valid_schema.clone(), true, drop_tx, diff --git a/apollo-router/src/plugins/response_cache/tests.rs b/apollo-router/src/plugins/response_cache/tests.rs index c1281812f9..561504f451 100644 --- a/apollo-router/src/plugins/response_cache/tests.rs +++ b/apollo-router/src/plugins/response_cache/tests.rs @@ -16,14 +16,17 @@ use super::plugin::ResponseCache; use crate::Context; use crate::MockedSubgraphs; use crate::TestHarness; +use crate::configuration::subgraph::SubgraphConfiguration; use crate::graphql; use crate::metrics::FutureMetricsExt; use crate::plugin::test::MockSubgraph; use crate::plugin::test::MockSubgraphService; use crate::plugins::response_cache::debugger::CacheKeysContext; use crate::plugins::response_cache::invalidation::InvalidationRequest; +use crate::plugins::response_cache::invalidation_endpoint::SubgraphInvalidationConfig; use crate::plugins::response_cache::plugin::CACHE_DEBUG_HEADER_NAME; use crate::plugins::response_cache::plugin::CONTEXT_CACHE_KEY; +use crate::plugins::response_cache::plugin::INVALIDATION_SHARED_KEY; use crate::plugins::response_cache::plugin::Subgraph; use crate::plugins::response_cache::storage::CacheStorage; use crate::plugins::response_cache::storage::redis::Config; @@ -60,6 +63,21 @@ async fn wait_for_cache(storage: &Storage, keys: Vec) { panic!("insert not complete"); } +pub(super) fn create_subgraph_conf( + subgraphs: HashMap, +) -> SubgraphConfiguration { + SubgraphConfiguration { + all: Subgraph { + invalidation: Some(SubgraphInvalidationConfig { + enabled: true, + shared_key: INVALIDATION_SHARED_KEY.to_string(), + }), + ..Default::default() + }, + subgraphs, + } +} + /// Extracts a list of cache keys from `CacheKeysContext` that we expect to be cached. This is /// mostly used in `wait_for_cache`. /// @@ -165,34 +183,41 @@ async fn insert() { let storage = Storage::new(&Config::test(false, "test_insert_simple"), drop_rx) .await .unwrap(); - let map = [ - ( - "user".to_string(), - Subgraph { - redis: None, - private_id: Some("sub".to_string()), - enabled: true.into(), - ttl: None, - ..Default::default() - }, - ), - ( - "orga".to_string(), - Subgraph { - redis: None, - private_id: Some("sub".to_string()), - enabled: true.into(), - ttl: None, - ..Default::default() - }, - ), - ] - .into_iter() - .collect(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf( + [ + ( + "user".to_string(), + Subgraph { + redis: None, + private_id: Some("sub".to_string()), + enabled: true.into(), + ttl: None, + ..Default::default() + }, + ), + ( + "orga".to_string(), + Subgraph { + redis: None, + private_id: Some("sub".to_string()), + enabled: true.into(), + ttl: None, + ..Default::default() + }, + ), + ] + .into_iter() + .collect(), + ); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({ @@ -356,10 +381,16 @@ async fn insert_with_custom_key() { ] .into_iter() .collect(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({ @@ -539,10 +570,16 @@ async fn already_expired_cache_control() { ] .into_iter() .collect(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({ @@ -701,10 +738,16 @@ async fn insert_without_debug_header() { ] .into_iter() .collect(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({ @@ -855,9 +898,10 @@ async fn insert_with_requires() { ] .into_iter() .collect(); + let subgraphs_conf = create_subgraph_conf(map); let response_cache = ResponseCache::for_test( storage.clone(), - map.clone(), + subgraphs_conf, valid_schema.clone(), true, drop_tx, @@ -1022,10 +1066,16 @@ async fn insert_with_nested_field_set() { ] .into_iter() .collect(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true }, "experimental_mock_subgraphs": subgraphs.clone() })) @@ -1168,7 +1218,7 @@ async fn no_cache_control() { .unwrap(); let response_cache = ResponseCache::for_test( storage.clone(), - HashMap::new(), + Default::default(), valid_schema.clone(), false, drop_tx, @@ -1295,7 +1345,7 @@ async fn no_store_from_request() { .unwrap(); let response_cache = ResponseCache::for_test( storage.clone(), - HashMap::new(), + Default::default(), valid_schema.clone(), false, drop_tx, @@ -1494,8 +1544,10 @@ async fn private_only() { ] .into_iter() .collect(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) + ResponseCache::for_test(storage.clone(), subgraphs_conf, valid_schema.clone(), true, drop_tx) .await .unwrap(); @@ -1696,10 +1748,16 @@ async fn private_and_public() { ] .into_iter() .collect(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let mut service = TestHarness::builder() .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true }, "experimental_mock_subgraphs": subgraphs.clone() })) @@ -1905,8 +1963,9 @@ async fn polymorphic_private_and_public() { ] .into_iter() .collect(); + let subgraphs_conf = create_subgraph_conf(map); let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) + ResponseCache::for_test(storage.clone(), subgraphs_conf, valid_schema.clone(), true, drop_tx) .await .unwrap(); @@ -2290,8 +2349,10 @@ async fn private_without_private_id() { ] .into_iter() .collect(); + + let subgraphs_conf = create_subgraph_conf(map); let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) + ResponseCache::for_test(storage.clone(), subgraphs_conf, valid_schema.clone(), true, drop_tx) .await .unwrap(); @@ -2463,11 +2524,16 @@ async fn no_data() { ] .into_iter() .collect(); - - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let subgraphs_conf = create_subgraph_conf(map); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } })) @@ -2699,15 +2765,21 @@ async fn missing_entities() { ] .into_iter() .collect(); + let subgraphs_conf = create_subgraph_conf(map); let (drop_tx, drop_rx) = tokio::sync::broadcast::channel(2); let storage = Storage::new(&Config::test(false, "missing_entities"), drop_rx) .await .unwrap(); - let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) - .await - .unwrap(); + let response_cache = ResponseCache::for_test( + storage.clone(), + subgraphs_conf, + valid_schema.clone(), + true, + drop_tx, + ) + .await + .unwrap(); let service = TestHarness::builder() .configuration_json(serde_json::json!({"include_subgraph_errors": { "all": true } })) @@ -2739,7 +2811,7 @@ async fn missing_entities() { .unwrap(); let response_cache = ResponseCache::for_test( storage.clone(), - HashMap::new(), + Default::default(), valid_schema.clone(), false, drop_tx, @@ -2871,8 +2943,9 @@ async fn invalidate_by_cache_tag() { ] .into_iter() .collect(); + let subgraphs_conf = create_subgraph_conf(map); let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) + ResponseCache::for_test(storage.clone(), subgraphs_conf, valid_schema.clone(), true, drop_tx) .await .unwrap(); @@ -3086,8 +3159,9 @@ async fn invalidate_by_type() { ] .into_iter() .collect(); + let subgraphs_conf = create_subgraph_conf(map); let response_cache = - ResponseCache::for_test(storage.clone(), map, valid_schema.clone(), true, drop_tx) + ResponseCache::for_test(storage.clone(), subgraphs_conf, valid_schema.clone(), true, drop_tx) .await .unwrap(); From ac74238cab0b1133e59c1961a110ece867c8b7c9 Mon Sep 17 00:00:00 2001 From: Benjamin <5719034+bnjjj@users.noreply.github.com> Date: Thu, 27 Nov 2025 16:16:19 +0100 Subject: [PATCH 2/3] changelog Signed-off-by: Benjamin <5719034+bnjjj@users.noreply.github.com> --- .../fix_response_cache_redis_requirement.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 .changesets/fix_response_cache_redis_requirement.md diff --git a/.changesets/fix_response_cache_redis_requirement.md b/.changesets/fix_response_cache_redis_requirement.md new file mode 100644 index 0000000000..16cafd906a --- /dev/null +++ b/.changesets/fix_response_cache_redis_requirement.md @@ -0,0 +1,45 @@ +### Validate Redis configuration based on response cache enabled state ([PR #8684](https://github.com/apollographql/router/pull/8684)) + +Previously, the router would attempt to connect to Redis for response caching regardless of whether response caching was enabled or disabled. This could cause unnecessary connection attempts and configuration errors even when the feature was explicitly disabled. + +Now, the router properly validates Redis configuration based on the response cache state: + +**When response caching is disabled**: Redis configuration is not required and no connection attempts are made. + +**When response caching is enabled**: Redis configuration is validated and required. If a subgraph has caching enabled but no Redis configuration, the router will return a clear error: + +``` +Error: you must have a redis configured either for all subgraphs or for subgraph "products" +``` + +This validation ensures that: +- You can disable response caching without needing to provide Redis configuration +- When response caching is enabled, all enabled subgraphs have proper Redis connectivity (either via global `all` configuration or per-subgraph configuration) +- Configuration errors are caught at startup with clear error messages + +Example configuration that now works correctly: + +```yaml +response_cache: + enabled: false # Redis not required when disabled + # … + subgraph: + all: + # … + enabled: false +``` + +```yaml +response_cache: + enabled: true + # … + subgraph: + all: + enabled: true + # … + redis: + urls: + - redis://127.0.0.1:6379 # Required when enabled +``` + +By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/8684 From c8576fed4949d5bbcddbb9d71b94dd2cc53d4b55 Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Fri, 28 Nov 2025 08:28:07 +0100 Subject: [PATCH 3/3] Update fix_response_cache_redis_requirement.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Renée --- .../fix_response_cache_redis_requirement.md | 41 ++----------------- 1 file changed, 3 insertions(+), 38 deletions(-) diff --git a/.changesets/fix_response_cache_redis_requirement.md b/.changesets/fix_response_cache_redis_requirement.md index 16cafd906a..a211a082ca 100644 --- a/.changesets/fix_response_cache_redis_requirement.md +++ b/.changesets/fix_response_cache_redis_requirement.md @@ -2,44 +2,9 @@ Previously, the router would attempt to connect to Redis for response caching regardless of whether response caching was enabled or disabled. This could cause unnecessary connection attempts and configuration errors even when the feature was explicitly disabled. -Now, the router properly validates Redis configuration based on the response cache state: +Now, the router ignores Redis configuration if response caching is disabled. +If response caching is configured to be _enabled_, Redis configuration is required, and missing Redis configuration will raise an error on startup: -**When response caching is disabled**: Redis configuration is not required and no connection attempts are made. - -**When response caching is enabled**: Redis configuration is validated and required. If a subgraph has caching enabled but no Redis configuration, the router will return a clear error: - -``` -Error: you must have a redis configured either for all subgraphs or for subgraph "products" -``` - -This validation ensures that: -- You can disable response caching without needing to provide Redis configuration -- When response caching is enabled, all enabled subgraphs have proper Redis connectivity (either via global `all` configuration or per-subgraph configuration) -- Configuration errors are caught at startup with clear error messages - -Example configuration that now works correctly: - -```yaml -response_cache: - enabled: false # Redis not required when disabled - # … - subgraph: - all: - # … - enabled: false -``` - -```yaml -response_cache: - enabled: true - # … - subgraph: - all: - enabled: true - # … - redis: - urls: - - redis://127.0.0.1:6379 # Required when enabled -``` +> Error: you must have a redis configured either for all subgraphs or for subgraph "products" By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/8684