Skip to content

Commit e39447b

Browse files
abernixabernix
committed
Merge branch 'dev' into i1g/max_headers
2 parents 22b8239 + b09bda0 commit e39447b

File tree

179 files changed

+2276
-558
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

179 files changed

+2276
-558
lines changed

‎.changesets/fix_renee_limit_errors.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
### Limit the amount of GraphQL validation errors returned in the response ([PR #6187](https://github.com/apollographql/router/pull/6187))
2+
3+
When an invalid query is submitted, the router now returns at most 100 GraphQL parsing and validation errors in the response.
4+
This prevents generating a very large response for nonsense documents.
5+
6+
By [@goto-bus-stop](https://github.com/goto-bus-stop) in https://github.com/apollographql/router/pull/6187

‎.changesets/helm_host_configuration.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
### Allow for configuration of the host via the helm template for virtual service ([PR #5545](https://github.com/apollographql/router/pull/5795))
2+
3+
Using the virtual service template change allows the configuration of the host from a variable when doing helm deploy.
4+
The default of any host causes issues for those that use different hosts for a single AKS cluster
5+
6+
By [@nicksephora](https://github.com/nicksephora) in https://github.com/apollographql/router/pull/5545

‎apollo-federation/src/error/mod.rs

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,44 @@ use lazy_static::lazy_static;
1313

1414
use crate::subgraph::spec::FederationSpecError;
1515

16-
/// Break out of the current function, returning an internal error.
16+
/// Create an internal error.
17+
///
18+
/// # Example
19+
/// ```rust
20+
/// use apollo_federation::internal_error;
21+
/// use apollo_federation::error::FederationError;
22+
/// # fn may_be_none() -> Option<()> { None }
23+
///
24+
/// const NAME: &str = "the thing";
25+
/// let result: Result<(), FederationError> = may_be_none()
26+
/// .ok_or_else(|| internal_error!("Expected {NAME} to be Some"));
27+
/// ```
1728
#[macro_export]
1829
macro_rules! internal_error {
1930
( $( $arg:tt )+ ) => {
20-
return Err($crate::error::FederationError::internal(format!( $( $arg )+ )).into());
31+
$crate::error::FederationError::internal(format!( $( $arg )+ ))
32+
}
33+
}
34+
35+
/// Break out of the current function, returning an internal error.
36+
///
37+
/// # Example
38+
/// ```rust
39+
/// use apollo_federation::bail;
40+
/// use apollo_federation::error::FederationError;
41+
/// # fn may_be_none() -> Option<()> { None }
42+
///
43+
/// fn example() -> Result<(), FederationError> {
44+
/// bail!("Something went horribly wrong");
45+
/// unreachable!()
46+
/// }
47+
/// #
48+
/// # _ = example();
49+
/// ```
50+
#[macro_export]
51+
macro_rules! bail {
52+
( $( $arg:tt )+ ) => {
53+
return Err($crate::internal_error!( $( $arg )+ ).into());
2154
}
2255
}
2356

@@ -26,6 +59,18 @@ macro_rules! internal_error {
2659
///
2760
/// Treat this as an assertion. It must only be used for conditions that *should never happen*
2861
/// in normal operation.
62+
///
63+
/// # Example
64+
/// ```rust,no_run
65+
/// use apollo_federation::ensure;
66+
/// use apollo_federation::error::FederationError;
67+
/// # fn may_be_none() -> Option<()> { None }
68+
///
69+
/// fn example() -> Result<(), FederationError> {
70+
/// ensure!(1 == 0, "Something went horribly wrong");
71+
/// unreachable!()
72+
/// }
73+
/// ```
2974
#[macro_export]
3075
macro_rules! ensure {
3176
( $expr:expr, $( $arg:tt )+ ) => {

‎apollo-federation/src/operation/merging.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ use super::NamedFragments;
1515
use super::Selection;
1616
use super::SelectionSet;
1717
use super::SelectionValue;
18+
use crate::bail;
1819
use crate::ensure;
1920
use crate::error::FederationError;
20-
use crate::internal_error;
2121

2222
impl<'a> FieldSelectionValue<'a> {
2323
/// Merges the given field selections into this one.
@@ -50,14 +50,14 @@ impl<'a> FieldSelectionValue<'a> {
5050
);
5151
if self.get().selection_set.is_some() {
5252
let Some(other_selection_set) = &other.selection_set else {
53-
internal_error!(
53+
bail!(
5454
"Field \"{}\" has composite type but not a selection set",
5555
other_field.field_position,
5656
);
5757
};
5858
selection_sets.push(other_selection_set);
5959
} else if other.selection_set.is_some() {
60-
internal_error!(
60+
bail!(
6161
"Field \"{}\" has non-composite type but also has a selection set",
6262
other_field.field_position,
6363
);
@@ -187,7 +187,7 @@ impl SelectionSet {
187187
selection_map::Entry::Occupied(existing) => match existing.get() {
188188
Selection::Field(self_field_selection) => {
189189
let Selection::Field(other_field_selection) = other_selection else {
190-
internal_error!(
190+
bail!(
191191
"Field selection key for field \"{}\" references non-field selection",
192192
self_field_selection.field.field_position,
193193
);
@@ -201,7 +201,7 @@ impl SelectionSet {
201201
let Selection::FragmentSpread(other_fragment_spread_selection) =
202202
other_selection
203203
else {
204-
internal_error!(
204+
bail!(
205205
"Fragment spread selection key for fragment \"{}\" references non-field selection",
206206
self_fragment_spread_selection.spread.fragment_name,
207207
);
@@ -215,7 +215,7 @@ impl SelectionSet {
215215
let Selection::InlineFragment(other_inline_fragment_selection) =
216216
other_selection
217217
else {
218-
internal_error!(
218+
bail!(
219219
"Inline fragment selection key under parent type \"{}\" {}references non-field selection",
220220
self_inline_fragment_selection.inline_fragment.parent_type_position,
221221
self_inline_fragment_selection.inline_fragment.type_condition_position.clone()
@@ -368,7 +368,7 @@ pub(crate) fn merge_selection_sets(
368368
mut selection_sets: Vec<SelectionSet>,
369369
) -> Result<SelectionSet, FederationError> {
370370
let Some((first, remainder)) = selection_sets.split_first_mut() else {
371-
internal_error!("merge_selection_sets(): must have at least one selection set");
371+
bail!("merge_selection_sets(): must have at least one selection set");
372372
};
373373
first.merge_into(remainder.iter())?;
374374

‎apollo-federation/src/operation/simplify.rs

Lines changed: 23 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -348,15 +348,29 @@ impl InlineFragmentSelection {
348348
} else {
349349
let rebased_inline_fragment = self.inline_fragment.rebase_on(parent_type, schema)?;
350350
let rebased_casted_type = rebased_inline_fragment.casted_type();
351-
let rebased_selection_set =
352-
selection_set.rebase_on(&rebased_casted_type, named_fragments, schema)?;
353-
Ok(Some(
354-
Selection::InlineFragment(Arc::new(InlineFragmentSelection::new(
355-
rebased_inline_fragment,
356-
rebased_selection_set,
357-
)))
358-
.into(),
359-
))
351+
// Re-flatten with the rebased casted type, which could further flatten away.
352+
let selection_set = selection_set.flatten_unnecessary_fragments(
353+
&rebased_casted_type,
354+
named_fragments,
355+
schema,
356+
)?;
357+
if selection_set.is_empty() {
358+
Ok(None)
359+
} else {
360+
// We need to rebase since the parent type for the selection set could be
361+
// changed.
362+
// Note: Rebasing after flattening, since rebasing before that can error out.
363+
// Or, `flatten_unnecessary_fragments` could `rebase` at the same time.
364+
let rebased_selection_set =
365+
selection_set.rebase_on(&rebased_casted_type, named_fragments, schema)?;
366+
Ok(Some(
367+
Selection::InlineFragment(Arc::new(InlineFragmentSelection::new(
368+
rebased_inline_fragment,
369+
rebased_selection_set,
370+
)))
371+
.into(),
372+
))
373+
}
360374
}
361375
}
362376
}

‎apollo-federation/src/query_graph/mod.rs

Lines changed: 20 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use petgraph::Direction;
1616

1717
use crate::error::FederationError;
1818
use crate::error::SingleFederationError;
19+
use crate::internal_error;
1920
use crate::operation::Field;
2021
use crate::operation::InlineFragment;
2122
use crate::operation::SelectionSet;
@@ -368,39 +369,27 @@ impl QueryGraph {
368369
}
369370

370371
pub(crate) fn node_weight(&self, node: NodeIndex) -> Result<&QueryGraphNode, FederationError> {
371-
self.graph.node_weight(node).ok_or_else(|| {
372-
SingleFederationError::Internal {
373-
message: "Node unexpectedly missing".to_owned(),
374-
}
375-
.into()
376-
})
372+
self.graph
373+
.node_weight(node)
374+
.ok_or_else(|| internal_error!("Node unexpectedly missing"))
377375
}
378376

379377
fn node_weight_mut(&mut self, node: NodeIndex) -> Result<&mut QueryGraphNode, FederationError> {
380-
self.graph.node_weight_mut(node).ok_or_else(|| {
381-
SingleFederationError::Internal {
382-
message: "Node unexpectedly missing".to_owned(),
383-
}
384-
.into()
385-
})
378+
self.graph
379+
.node_weight_mut(node)
380+
.ok_or_else(|| internal_error!("Node unexpectedly missing"))
386381
}
387382

388383
pub(crate) fn edge_weight(&self, edge: EdgeIndex) -> Result<&QueryGraphEdge, FederationError> {
389-
self.graph.edge_weight(edge).ok_or_else(|| {
390-
SingleFederationError::Internal {
391-
message: "Edge unexpectedly missing".to_owned(),
392-
}
393-
.into()
394-
})
384+
self.graph
385+
.edge_weight(edge)
386+
.ok_or_else(|| internal_error!("Edge unexpectedly missing"))
395387
}
396388

397389
fn edge_weight_mut(&mut self, edge: EdgeIndex) -> Result<&mut QueryGraphEdge, FederationError> {
398-
self.graph.edge_weight_mut(edge).ok_or_else(|| {
399-
SingleFederationError::Internal {
400-
message: "Edge unexpectedly missing".to_owned(),
401-
}
402-
.into()
403-
})
390+
self.graph
391+
.edge_weight_mut(edge)
392+
.ok_or_else(|| internal_error!("Edge unexpectedly missing"))
404393
}
405394

406395
pub(crate) fn edge_head_weight(
@@ -415,12 +404,9 @@ impl QueryGraph {
415404
&self,
416405
edge: EdgeIndex,
417406
) -> Result<(NodeIndex, NodeIndex), FederationError> {
418-
self.graph.edge_endpoints(edge).ok_or_else(|| {
419-
SingleFederationError::Internal {
420-
message: "Edge unexpectedly missing".to_owned(),
421-
}
422-
.into()
423-
})
407+
self.graph
408+
.edge_endpoints(edge)
409+
.ok_or_else(|| internal_error!("Edge unexpectedly missing"))
424410
}
425411

426412
fn schema(&self) -> Result<&ValidFederationSchema, FederationError> {
@@ -431,12 +417,9 @@ impl QueryGraph {
431417
&self,
432418
source: &str,
433419
) -> Result<&ValidFederationSchema, FederationError> {
434-
self.sources.get(source).ok_or_else(|| {
435-
SingleFederationError::Internal {
436-
message: "Schema unexpectedly missing".to_owned(),
437-
}
438-
.into()
439-
})
420+
self.sources
421+
.get(source)
422+
.ok_or_else(|| internal_error!(r#"Schema for "{source}" unexpectedly missing"#))
440423
}
441424

442425
pub(crate) fn subgraph_schemas(&self) -> &IndexMap<Arc<str>, ValidFederationSchema> {
@@ -454,7 +437,7 @@ impl QueryGraph {
454437
) -> Result<&IndexSet<NodeIndex>, FederationError> {
455438
self.types_to_nodes()?
456439
.get(name)
457-
.ok_or_else(|| FederationError::internal("No nodes unexpectedly found for type"))
440+
.ok_or_else(|| internal_error!("No nodes unexpectedly found for type"))
458441
}
459442

460443
pub(crate) fn types_to_nodes(

‎apollo-federation/src/query_plan/conditions.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ use apollo_compiler::Node;
88
use indexmap::map::Entry;
99
use serde::Serialize;
1010

11+
use crate::bail;
1112
use crate::error::FederationError;
12-
use crate::internal_error;
1313
use crate::operation::DirectiveList;
1414
use crate::operation::NamedFragments;
1515
use crate::operation::Selection;
@@ -122,7 +122,7 @@ impl Conditions {
122122

123123
if let Some(skip) = directives.get("skip") {
124124
let Some(value) = skip.specified_argument_by_name("if") else {
125-
internal_error!("missing @skip(if:) argument");
125+
bail!("missing @skip(if:) argument");
126126
};
127127

128128
match value.as_ref() {
@@ -134,14 +134,14 @@ impl Conditions {
134134
variables.insert(name.clone(), ConditionKind::Skip);
135135
}
136136
_ => {
137-
internal_error!("expected boolean or variable `if` argument, got {value}");
137+
bail!("expected boolean or variable `if` argument, got {value}");
138138
}
139139
}
140140
}
141141

142142
if let Some(include) = directives.get("include") {
143143
let Some(value) = include.specified_argument_by_name("if") else {
144-
internal_error!("missing @include(if:) argument");
144+
bail!("missing @include(if:) argument");
145145
};
146146

147147
match value.as_ref() {
@@ -159,7 +159,7 @@ impl Conditions {
159159
}
160160
}
161161
_ => {
162-
internal_error!("expected boolean or variable `if` argument, got {value}");
162+
bail!("expected boolean or variable `if` argument, got {value}");
163163
}
164164
}
165165
}

‎apollo-federation/tests/query_plan/build_query_plan_support.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ use apollo_federation::query_plan::TopLevelPlanNode;
1212
use apollo_federation::schema::ValidFederationSchema;
1313
use sha1::Digest;
1414

15-
const ROVER_FEDERATION_VERSION: &str = "2.7.4";
15+
const ROVER_FEDERATION_VERSION: &str = "2.9.0";
1616

17-
const DEFAULT_LINK_DIRECTIVE: &str = r#"@link(url: "https://specs.apollo.dev/federation/v2.7", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject"])"#;
17+
const DEFAULT_LINK_DIRECTIVE: &str = r#"@link(url: "https://specs.apollo.dev/federation/v2.9", import: ["@key", "@requires", "@provides", "@external", "@tag", "@extends", "@shareable", "@inaccessible", "@override", "@composeDirective", "@interfaceObject", "@context", "@fromContext", "@cost", "@listSize"])"#;
1818

1919
/// Runs composition on the given subgraph schemas and return `(api_schema, query_planner)`
2020
///

‎apollo-federation/tests/query_plan/build_query_plan_tests.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1361,3 +1361,53 @@ fn condition_order_router799() {
13611361
"###
13621362
);
13631363
}
1364+
1365+
#[test]
1366+
fn rebase_non_intersecting_without_dropping_inline_fragment_due_to_directive() {
1367+
let planner = planner!(
1368+
Subgraph1: r#"
1369+
type Query {
1370+
test: X
1371+
}
1372+
1373+
interface I {
1374+
i: Int
1375+
}
1376+
1377+
type X implements I {
1378+
i: Int
1379+
}
1380+
1381+
type Y implements I {
1382+
i: Int
1383+
}
1384+
"#,
1385+
);
1386+
assert_plan!(
1387+
&planner,
1388+
r#"
1389+
{
1390+
test { # type X
1391+
... on I { # upcast to I
1392+
... @skip(if: false) { # This fragment can't be dropped due to its directive.
1393+
... on Y { # downcast to Y (non-intersecting)
1394+
i
1395+
}
1396+
}
1397+
}
1398+
}
1399+
}
1400+
"#,
1401+
@r###"
1402+
QueryPlan {
1403+
Fetch(service: "Subgraph1") {
1404+
{
1405+
test {
1406+
__typename @include(if: false)
1407+
}
1408+
}
1409+
},
1410+
}
1411+
"###
1412+
);
1413+
}

0 commit comments

Comments
 (0)