diff --git a/.changesets/fix_caroline_rh_1292.md b/.changesets/fix_caroline_rh_1292.md new file mode 100644 index 0000000000..b5795d1697 --- /dev/null +++ b/.changesets/fix_caroline_rh_1292.md @@ -0,0 +1,9 @@ +### Handle both 'deprecated' enum values when merging coprocessor context ([PR #8997](https://github.com/apollographql/router/pull/8997)) + +A change to coprocessor context merges in Router v2.10 caused keys to be deleted when `context: true` is used as the coprocessor context selector in the router configuration file. + +The quick fix in the router config is to pass `context: deprecated` instead. + +This change brings parity when `context: true` is provided. + +By [@carodewig](https://github.com/carodewig) in https://github.com/apollographql/router/pull/8997 diff --git a/apollo-router/src/plugins/coprocessor/mod.rs b/apollo-router/src/plugins/coprocessor/mod.rs index 6c8c7ee6fc..bb67476305 100644 --- a/apollo-router/src/plugins/coprocessor/mod.rs +++ b/apollo-router/src/plugins/coprocessor/mod.rs @@ -437,6 +437,15 @@ pub(super) enum ContextConf { NewContextConf(NewContextConf), } +impl ContextConf { + fn is_deprecated(&self) -> bool { + match self { + Self::Deprecated(v) => *v, + Self::NewContextConf(c) => *c == NewContextConf::Deprecated, + } + } +} + impl Default for ContextConf { fn default() -> Self { Self::Deprecated(false) @@ -509,7 +518,7 @@ pub(crate) fn update_context_from_coprocessor( for (mut key, value) in context_returned.try_into_iter()? { // Handle deprecated key names - convert back to actual key names - if let ContextConf::NewContextConf(NewContextConf::Deprecated) = context_config { + if context_config.is_deprecated() { key = context_key_from_deprecated(key); } diff --git a/apollo-router/src/plugins/coprocessor/test.rs b/apollo-router/src/plugins/coprocessor/test.rs index 054269b985..229ec7b5dd 100644 --- a/apollo-router/src/plugins/coprocessor/test.rs +++ b/apollo-router/src/plugins/coprocessor/test.rs @@ -20,6 +20,7 @@ mod tests { use super::super::*; use crate::assert_response_eq_ignoring_error_id; + use crate::context::deprecated::DEPRECATED_CLIENT_NAME; use crate::graphql::Response; use crate::json_ext::Object; use crate::json_ext::Value; @@ -32,6 +33,7 @@ mod tests { use crate::plugins::coprocessor::supergraph::SupergraphResponseConf; use crate::plugins::coprocessor::supergraph::SupergraphStage; use crate::plugins::coprocessor::was_incoming_payload_valid; + use crate::plugins::telemetry::CLIENT_NAME; use crate::plugins::telemetry::config_new::conditions::SelectorOrValue; use crate::services::external::EXTERNALIZABLE_VERSION; use crate::services::external::Externalizable; @@ -4114,4 +4116,33 @@ mod tests { Some(serde_json_bytes::json!("preserved_value")) ); } + #[rstest::rstest] + fn test_update_context_from_coprocessor_handles_deprecated_key_names( + #[values(DEPRECATED_CLIENT_NAME, CLIENT_NAME)] target_context_key_name: &str, + #[values( + ContextConf::Deprecated(true), + ContextConf::NewContextConf(NewContextConf::Deprecated) + )] + context_conf: ContextConf, + ) { + use crate::Context; + use crate::plugins::coprocessor::update_context_from_coprocessor; + + let target_context = + Context::from_iter([(target_context_key_name.to_string(), "v1".into())]); + let returned_context = + Context::from_iter([(DEPRECATED_CLIENT_NAME.to_string(), "v2".into())]); + + update_context_from_coprocessor(&target_context, returned_context, &context_conf).unwrap(); + + assert_eq!( + target_context.get_json_value(CLIENT_NAME), + Some(json!("v2")), + ); + + assert!( + !target_context.contains_key(DEPRECATED_CLIENT_NAME), + "DEPRECATED_CLIENT_NAME should not be present" + ); + } }