Skip to content

Commit e34906b

Browse files
authored
Fix: Traffic shaping configuration fallback for experimental_enable_http2 (#2976)
Fix a bug where `experimental_enable_http2` wouldn't properly apply when a global configuration was set.
1 parent 6cc999b commit e34906b

File tree

2 files changed

+79
-5
lines changed

2 files changed

+79
-5
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
### fix: Traffic shaping configuration fallback for experimental_enable_http2
2+
3+
Fix a bug where experimental_enable_http2 wouldn't properly apply when a global configuration was set.
4+
5+
Huge thanks to @westhechiang, @leggomuhgreggo @vecchp and @davidvasandani for discovering the issue and finding a reproducible testcase!
6+
7+
By [@o0Ignition0o](https://github.com/o0Ignition0o) in https://github.com/apollographql/router/pull/2976

apollo-router/src/plugins/traffic_shaping/mod.rs

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -451,11 +451,12 @@ impl TrafficShaping {
451451
}
452452

453453
pub(crate) fn enable_subgraph_http2(&self, service_name: &str) -> bool {
454-
self.config
455-
.subgraphs
456-
.get(service_name)
457-
.and_then(|subgraph| subgraph.shaping.experimental_enable_http2)
458-
.unwrap_or(true)
454+
Self::merge_config(
455+
self.config.all.as_ref(),
456+
self.config.subgraphs.get(service_name),
457+
)
458+
.and_then(|config| config.shaping.experimental_enable_http2)
459+
.unwrap_or(true)
459460
}
460461
}
461462

@@ -692,6 +693,72 @@ mod test {
692693
);
693694
}
694695

696+
#[test]
697+
fn test_merge_http2_all() {
698+
let config = serde_yaml::from_str::<Config>(
699+
r#"
700+
all:
701+
experimental_enable_http2: false
702+
subgraphs:
703+
products:
704+
experimental_enable_http2: true
705+
reviews:
706+
experimental_enable_http2: false
707+
router:
708+
timeout: 65s
709+
"#,
710+
)
711+
.unwrap();
712+
713+
assert!(TrafficShaping::merge_config(
714+
config.all.as_ref(),
715+
config.subgraphs.get("products")
716+
)
717+
.unwrap()
718+
.shaping
719+
.experimental_enable_http2
720+
.unwrap());
721+
assert!(!TrafficShaping::merge_config(
722+
config.all.as_ref(),
723+
config.subgraphs.get("reviews")
724+
)
725+
.unwrap()
726+
.shaping
727+
.experimental_enable_http2
728+
.unwrap());
729+
assert!(!TrafficShaping::merge_config(config.all.as_ref(), None)
730+
.unwrap()
731+
.shaping
732+
.experimental_enable_http2
733+
.unwrap());
734+
}
735+
736+
#[tokio::test]
737+
async fn test_enable_subgraph_http2() {
738+
let config = serde_yaml::from_str::<Config>(
739+
r#"
740+
all:
741+
experimental_enable_http2: false
742+
subgraphs:
743+
products:
744+
experimental_enable_http2: true
745+
reviews:
746+
experimental_enable_http2: false
747+
router:
748+
timeout: 65s
749+
"#,
750+
)
751+
.unwrap();
752+
753+
let shaping_config = TrafficShaping::new(PluginInit::new(config, Default::default()))
754+
.await
755+
.unwrap();
756+
757+
assert!(shaping_config.enable_subgraph_http2("products"));
758+
assert!(!shaping_config.enable_subgraph_http2("reviews"));
759+
assert!(!shaping_config.enable_subgraph_http2("this_doesnt_exist"));
760+
}
761+
695762
#[tokio::test(flavor = "multi_thread")]
696763
async fn it_rate_limit_subgraph_requests() {
697764
let config = serde_yaml::from_str::<serde_json::Value>(

0 commit comments

Comments
 (0)