diff --git a/.changesets/maint_geal_always_create_traffic_shaping.md b/.changesets/maint_geal_always_create_traffic_shaping.md new file mode 100644 index 0000000000..2a53fd2903 --- /dev/null +++ b/.changesets/maint_geal_always_create_traffic_shaping.md @@ -0,0 +1,5 @@ +### Always instanciate the traffic shaping plugin ([Issue #3327](https://github.com/apollographql/router/issues/3327)) + +The traffic_shaping plugin is now always part of the plugins list and is always active, with default configuration. + +By [@Geal](https://github.com/Geal) in https://github.com/apollographql/router/pull/3330 \ No newline at end of file diff --git a/apollo-router/src/plugins/expose_query_plan.rs b/apollo-router/src/plugins/expose_query_plan.rs index d739913b01..d76fb2abef 100644 --- a/apollo-router/src/plugins/expose_query_plan.rs +++ b/apollo-router/src/plugins/expose_query_plan.rs @@ -122,7 +122,6 @@ register_plugin!("experimental", "expose_query_plan", ExposeQueryPlan); #[cfg(test)] mod tests { use once_cell::sync::Lazy; - use serde_json::Value as jValue; use serde_json_bytes::ByteString; use serde_json_bytes::Value; use tower::Service; @@ -131,9 +130,7 @@ mod tests { use crate::graphql::Response; use crate::json_ext::Object; use crate::plugin::test::MockSubgraph; - use crate::plugin::DynPlugin; - use crate::query_planner::BridgeQueryPlanner; - use crate::services::PluggableSupergraphServiceBuilder; + use crate::MockedSubgraphs; static EXPECTED_RESPONSE_WITH_QUERY_PLAN: Lazy = Lazy::new(|| { serde_json::from_str(include_str!( @@ -150,7 +147,7 @@ mod tests { static VALID_QUERY: &str = r#"query TopProducts($first: Int) { topProducts(first: $first) { upc name reviews { id product { name } author { id name } } } }"#; - async fn build_mock_supergraph(plugin: Box) -> supergraph::BoxService { + async fn build_mock_supergraph(config: serde_json::Value) -> supergraph::BoxCloneService { let mut extensions = Object::new(); extensions.insert("test", Value::String(ByteString::from("value"))); @@ -183,35 +180,32 @@ mod tests { let product_service = MockSubgraph::new(product_mocks).with_extensions(extensions); - let schema = - include_str!("../../../apollo-router-benchmarks/benches/fixtures/supergraph.graphql"); - let planner = BridgeQueryPlanner::new(schema.to_string(), Default::default()) - .await - .unwrap(); - - let builder = PluggableSupergraphServiceBuilder::new(planner); - let builder = builder - .with_dyn_plugin("experimental.expose_query_plan".to_string(), plugin) - .with_subgraph_service("accounts", account_service.clone()) - .with_subgraph_service("reviews", review_service.clone()) - .with_subgraph_service("products", product_service.clone()); - - builder.build().await.expect("should build").make().boxed() - } - - async fn get_plugin(config: &jValue) -> Box { - crate::plugin::plugins() - .find(|factory| factory.name == "experimental.expose_query_plan") - .expect("Plugin not found") - .create_instance_without_schema(config) + let subgraphs = MockedSubgraphs( + [ + ("accounts", account_service), + ("reviews", review_service), + ("products", product_service), + ] + .into_iter() + .collect(), + ); + + crate::TestHarness::builder() + .schema(include_str!( + "../../../apollo-router-benchmarks/benches/fixtures/supergraph.graphql" + )) + .extra_plugin(subgraphs) + .configuration_json(config) + .unwrap() + .build_supergraph() .await - .expect("Plugin not created") + .unwrap() } async fn execute_supergraph_test( query: &str, body: &Response, - mut supergraph_service: supergraph::BoxService, + mut supergraph_service: supergraph::BoxCloneService, ) { let request = supergraph::Request::fake_builder() .query(query.to_string()) @@ -231,32 +225,47 @@ mod tests { .await .unwrap(); - assert_eq!(response, *body); + assert_eq!( + serde_json::to_string(&response).unwrap(), + serde_json::to_string(body).unwrap() + ); } #[tokio::test] async fn it_expose_query_plan() { - let plugin = get_plugin(&serde_json::json!(true)).await; execute_supergraph_test( VALID_QUERY, &EXPECTED_RESPONSE_WITH_QUERY_PLAN, - build_mock_supergraph(plugin).await, + build_mock_supergraph(serde_json::json! {{ + "plugins": { + "experimental.expose_query_plan": true + } + }}) + .await, ) .await; // let's try that again - let plugin = get_plugin(&serde_json::json!(true)).await; execute_supergraph_test( VALID_QUERY, &EXPECTED_RESPONSE_WITH_QUERY_PLAN, - build_mock_supergraph(plugin).await, + build_mock_supergraph(serde_json::json! {{ + "plugins": { + "experimental.expose_query_plan": true + } + }}) + .await, ) .await; } #[tokio::test] async fn it_doesnt_expose_query_plan() { - let plugin = get_plugin(&serde_json::json!(false)).await; - let supergraph = build_mock_supergraph(plugin).await; + let supergraph = build_mock_supergraph(serde_json::json! {{ + "plugins": { + "experimental.expose_query_plan": false + } + }}) + .await; execute_supergraph_test( VALID_QUERY, &EXPECTED_RESPONSE_WITHOUT_QUERY_PLAN, diff --git a/apollo-router/src/router_factory.rs b/apollo-router/src/router_factory.rs index 4b3fc4de82..853d8ae24c 100644 --- a/apollo-router/src/router_factory.rs +++ b/apollo-router/src/router_factory.rs @@ -9,7 +9,6 @@ use rustls::RootCertStore; use serde_json::Map; use serde_json::Value; use tower::service_fn; -use tower::util::Either; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -185,39 +184,29 @@ impl RouterSuperServiceFactory for YamlRouterFactory { .transpose()? .or_else(|| tls_root_store.clone()); - let subgraph_service = match plugins + let shaping = plugins .iter() .find(|i| i.0.as_str() == APOLLO_TRAFFIC_SHAPING) .and_then(|plugin| (*plugin.1).as_any().downcast_ref::()) - { - Some(shaping) => Either::A( - shaping.subgraph_service_internal( - name, - SubgraphService::new( - name, - configuration - .apq - .subgraph - .subgraphs - .get(name) - .map(|apq| apq.enabled) - .unwrap_or(configuration.apq.subgraph.all.enabled), - subgraph_root_store, - shaping.enable_subgraph_http2(name), - subscription_plugin_conf.clone(), - configuration.notify.clone(), - ), - ), - ), - None => Either::B(SubgraphService::new( + .expect("traffic shaping should always be part of the plugin list"); + + let subgraph_service = shaping.subgraph_service_internal( + name, + SubgraphService::new( name, - false, + configuration + .apq + .subgraph + .subgraphs + .get(name) + .map(|apq| apq.enabled) + .unwrap_or(configuration.apq.subgraph.all.enabled), subgraph_root_store, - true, + shaping.enable_subgraph_http2(name), subscription_plugin_conf.clone(), configuration.notify.clone(), - )), - }; + ), + ); builder = builder.with_subgraph_service(name, subgraph_service); } @@ -308,39 +297,29 @@ impl YamlRouterFactory { .transpose()? .or_else(|| tls_root_store.clone()); - let subgraph_service = match plugins + let shaping = plugins .iter() .find(|i| i.0.as_str() == APOLLO_TRAFFIC_SHAPING) .and_then(|plugin| (*plugin.1).as_any().downcast_ref::()) - { - Some(shaping) => Either::A( - shaping.subgraph_service_internal( - name, - SubgraphService::new( - name, - configuration - .apq - .subgraph - .subgraphs - .get(name) - .map(|apq| apq.enabled) - .unwrap_or(configuration.apq.subgraph.all.enabled), - subgraph_root_store, - shaping.enable_subgraph_http2(name), - subscription_plugin_conf.clone(), - configuration.notify.clone(), - ), - ), - ), - None => Either::B(SubgraphService::new( + .expect("traffic shaping should always be part of the plugin list"); + + let subgraph_service = shaping.subgraph_service_internal( + name, + SubgraphService::new( name, - false, + configuration + .apq + .subgraph + .subgraphs + .get(name) + .map(|apq| apq.enabled) + .unwrap_or(configuration.apq.subgraph.all.enabled), subgraph_root_store, - true, + shaping.enable_subgraph_http2(name), subscription_plugin_conf.clone(), configuration.notify.clone(), - )), - }; + ), + ); builder = builder.with_subgraph_service(name, subgraph_service); } @@ -511,7 +490,7 @@ pub(crate) async fn create_plugins( add_mandatory_apollo_plugin!("csrf"); add_mandatory_apollo_plugin!("headers"); add_mandatory_apollo_plugin!("telemetry"); - add_optional_apollo_plugin!("traffic_shaping"); + add_mandatory_apollo_plugin!("traffic_shaping"); add_optional_apollo_plugin!("forbid_mutations"); add_optional_apollo_plugin!("subscription"); add_optional_apollo_plugin!("override_subgraph_url"); diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index c3da8c7185..fc12af8243 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -11,7 +11,6 @@ use http::StatusCode; use indexmap::IndexMap; use router_bridge::planner::Planner; use tokio::sync::Mutex; -use tower::util::Either; use tower::BoxError; use tower::ServiceBuilder; use tower::ServiceExt; @@ -440,15 +439,14 @@ impl SupergraphCreator { .schema(self.schema.clone()) .build(); - let supergraph_service = match self + let shaping = self .plugins .iter() .find(|i| i.0.as_str() == APOLLO_TRAFFIC_SHAPING) .and_then(|plugin| plugin.1.as_any().downcast_ref::()) - { - Some(shaping) => Either::A(shaping.supergraph_service_internal(supergraph_service)), - None => Either::B(supergraph_service), - }; + .expect("traffic shaping should always be part of the plugin list"); + + let supergraph_service = shaping.supergraph_service_internal(supergraph_service); ServiceBuilder::new() .layer(content_negociation::SupergraphLayer::default())