From 7115030a090a9546c49b65a12ef720316845c63f Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Tue, 26 Apr 2022 11:48:59 -0500 Subject: [PATCH] feat: default to fed2 when @link is present (#1097) if a user does not specify federation_version: 2 and they have an @link directive in a single subgraph, we'll default to Federation 2 rather than the old default of Federation 1. --- Cargo.lock | 45 ++++++++++++++++- Cargo.toml | 6 ++- crates/rover-client/Cargo.toml | 3 +- docs/source/supergraphs.md | 9 +++- src/command/supergraph/compose/do_compose.rs | 3 +- src/command/supergraph/resolve_config.rs | 53 ++++++++++++++++++-- src/utils/parsers.rs | 15 +++++- 7 files changed, 124 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f2caa585e..4405e8561 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -64,9 +64,9 @@ checksum = "c2ae23d8a1f5cc1a4e33ec9b1f0d4d626f9e4736619b4c1a4f1482a594ac2e13" [[package]] name = "apollo-federation-types" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c9e00aacd39acc78da616721350dd9cadcec8e6c1f9ac6f48f430e2ce2a21baf" +checksum = "871b2f209bdf216bacfc97e2c02afaa4c5931dec471aaaa6e38c7341529ee6d7" dependencies = [ "camino", "log", @@ -79,6 +79,15 @@ dependencies = [ "url", ] +[[package]] +name = "apollo-parser" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb3f8fe8c9fb08c1e99bf4a50698adcd807cafba2d2aacc49d8ffc79123fce59" +dependencies = [ + "rowan", +] + [[package]] name = "arrayref" version = "0.3.6" @@ -746,6 +755,12 @@ version = "0.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5827cebf4670468b8772dd191856768aedcb1b0278a04f989f7766351917b9dc" +[[package]] +name = "countme" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "328b822bdcba4d4e402be8d9adb6eebf269f969f8eadef977a553ff3c4fbcb58" + [[package]] name = "cpufeatures" version = "0.2.2" @@ -2624,6 +2639,7 @@ dependencies = [ "ansi_term", "anyhow", "apollo-federation-types", + "apollo-parser", "assert-json-diff", "assert_cmd", "assert_fs", @@ -2697,6 +2713,19 @@ dependencies = [ "uuid", ] +[[package]] +name = "rowan" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4a938f42b9c73aeece236481f37adb3debb7dfe3ae347cd6a45b5797d9ce4250" +dependencies = [ + "countme", + "hashbrown", + "memoffset", + "rustc-hash", + "text-size", +] + [[package]] name = "rust-argon2" version = "0.8.3" @@ -2715,6 +2744,12 @@ version = "0.1.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7ef03e0a2b150c7a90d01faf6254c9c48a41e95fb2a8c2ac1c6f0d2b9aefc342" +[[package]] +name = "rustc-hash" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" + [[package]] name = "rustversion" version = "1.0.6" @@ -3254,6 +3289,12 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "507e9898683b6c43a9aa55b64259b721b52ba226e0f3779137e50ad114a4c90b" +[[package]] +name = "text-size" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "288cb548dbe72b652243ea797201f3d481a0609a967980fcc5b2315ea811560a" + [[package]] name = "textwrap" version = "0.11.0" diff --git a/Cargo.toml b/Cargo.toml index 91ee2d1d3..13d3446e4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,8 +41,12 @@ composition-js = [] [dependencies] # https://github.com/apollographql/federation-rs dependencies -apollo-federation-types = "0.4" +apollo-federation-types = "0.5" # apollo-federation-types = { path = "../federation-rs/apollo-federation-types" } +# apollo-federation-types = { git = "https://github.com/apollographql/federation-rs", branch = "main" } + +# https://github.com/apollographql/apollo-rs dependencies +apollo-parser = "0.2" # workspace dependencies binstall = { path = "./installers/binstall" } diff --git a/crates/rover-client/Cargo.toml b/crates/rover-client/Cargo.toml index 22ed18adf..0f1ca4915 100644 --- a/crates/rover-client/Cargo.toml +++ b/crates/rover-client/Cargo.toml @@ -13,8 +13,9 @@ publish = false houston = { path = "../houston" } # https://github.com/apollographql/federation-rs dependencies -apollo-federation-types = "0.4" +apollo-federation-types = "0.5" # apollo-federation-types = { path = "../../../federation-rs/apollo-federation-types" } +# apollo-federation-types = { git = "https://github.com/apollographql/federation-rs", branch = "main" } # crates.io deps backoff = "0.4" diff --git a/docs/source/supergraphs.md b/docs/source/supergraphs.md index 520339f73..61601acd3 100644 --- a/docs/source/supergraphs.md +++ b/docs/source/supergraphs.md @@ -105,9 +105,14 @@ rover supergraph compose --config ./supergraph.yaml > prod-schema.graphql **Apollo Gateway fails to start up if it's provided with a supergraph schema that it doesn't support.** To ensure compatibility, we recommend that you test launching your gateway in a CI pipeline with the supergraph schema it will ultimately use in production. +#### Federation 1 vs. Federation 2 + The `rover supergraph compose` command generates a supergraph schema via [composition](https://www.apollographql.com/docs/federation/federated-types/composition/). For Federation 1, this algorithm was implemented in the [`@apollo/federation`](https://www.npmjs.com/package/@apollo/federation) package. For Federation 2, this algorithm is implemented in the [`@apollo/composition`](https://www.npmjs.com/package/@apollo/composition) package. -`rover supergraph compose` supports composing subgraphs with both Federation 1 and Federation 2. It is recommended that you set `federation_version: 1` or `federation_version: 2` in your YAML configuration file. When Apollo releases new versions of composition for Federation 1 or Federation 2, Rover finds the new package and downloads it to your machine. It then uses the new composition package to compose your subgraphs. Our aim is to release only backward-compatible changes across major versions moving forward. If composition breaks from one version to the next, please [submit an issue](https://github.com/apollographql/federation/issues/new?assignees=&labels=&template=bug.md), and follow the instructions for pinning composition to a known version. + +`rover supergraph compose` supports composing subgraphs with both Federation 1 and Federation 2. By default, Rover uses Federation 2 composition _if and only if_ at least one of your subgraph schemas contains the `@link` directive. Otherwise, it uses Federation 1. + +When Apollo releases new versions of composition for Federation 1 or Federation 2, Rover downloads the new package to your machine. Rover then uses the new composition package to compose your subgraphs. Our aim is to release only backward-compatible changes across major versions moving forward. If composition breaks from one version to the next, please [submit an issue](https://github.com/apollographql/federation/issues/new?assignees=&labels=&template=bug.md), and follow the instructions for pinning composition to a known good version. > **⚠️ If you need to pin your composition function to a specific version _(not recommended)_**, you can do so by setting `federation_version: =2.0.1` in your `supergraph.yaml` file. This ensures that Rover _always_ uses the exact version of composition that you specified. In this example, Rover would use `@apollo/composition@v2.0.1`. @@ -117,6 +122,8 @@ The `rover supergraph compose` command generates a supergraph schema via [compos If you set `federation_version: 1` or `federation_version: 2`, you can run `rover supergraph compose` with the `--skip-update` flag to prevent Rover from downloading newer composition versions. Rover instead uses the latest major version that you've downloaded to your machine. This can be helpful if you're on a slow network. +If any subgraph schema contains the `@link` directive and you've specified `federation_version: 1`, you either need to revert that subgraph to a Federation 1 schema or move your graph to Federation 2. + #### Earlier Rover versions Prior to Rover v0.5.0, `rover supergraph compose` shipped with exactly one version of composition that was compatible with Federation 1. This function was sourced from the [`@apollo/federation`](https://www.npmjs.com/package/@apollo/federation) JavaScript package. Therefore, it was important to keep track of your Rover version and your Gateway version and keep them in sync according to the following compatibility table. diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index 667fa1e4b..2f62c292f 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -56,7 +56,8 @@ impl Compose { &self.profile_name, )?; // first, grab the _actual_ federation version from the config we just resolved - let federation_version = supergraph_config.get_federation_version(); + // (this will always be `Some` as long as we have created with `resolve_supergraph_yaml` so it is safe to unwrap) + let federation_version = supergraph_config.get_federation_version().unwrap(); // and create our plugin that we may need to install from it let plugin = Plugin::Supergraph(federation_version.clone()); let plugin_name = plugin.get_name(); diff --git a/src/command/supergraph/resolve_config.rs b/src/command/supergraph/resolve_config.rs index 6c211d919..842482c40 100644 --- a/src/command/supergraph/resolve_config.rs +++ b/src/command/supergraph/resolve_config.rs @@ -1,7 +1,8 @@ use apollo_federation_types::{ build::SubgraphDefinition, - config::{SchemaSource, SupergraphConfig}, + config::{FederationVersion, SchemaSource, SupergraphConfig}, }; +use apollo_parser::{ast, Parser}; use std::{collections::HashMap, fs, str::FromStr}; @@ -29,7 +30,7 @@ pub(crate) fn resolve_supergraph_yaml( let contents = unresolved_supergraph_yaml .read_file_descriptor("supergraph config", &mut std::io::stdin())?; let supergraph_config = SupergraphConfig::new_from_yaml(&contents)?; - let federation_version = supergraph_config.get_federation_version(); + let maybe_specified_federation_version = supergraph_config.get_federation_version(); for (subgraph_name, subgraph_data) in supergraph_config.into_iter() { match &subgraph_data.schema { @@ -129,6 +130,52 @@ pub(crate) fn resolve_supergraph_yaml( } let mut resolved_supergraph_config: SupergraphConfig = subgraph_definitions.into(); - resolved_supergraph_config.set_federation_version(federation_version); + + let mut fed_two_subgraph_names = Vec::new(); + for subgraph_definition in resolved_supergraph_config.get_subgraph_definitions()? { + let parser = Parser::new(&subgraph_definition.sdl); + let parsed_ast = parser.parse(); + let doc = parsed_ast.document(); + for definition in doc.definitions() { + let maybe_directives = match definition { + ast::Definition::SchemaExtension(ext) => ext.directives(), + ast::Definition::SchemaDefinition(def) => def.directives(), + _ => None, + } + .map(|d| d.directives()); + if let Some(directives) = maybe_directives { + for directive in directives { + if let Some(directive_name) = directive.name() { + if "link" == directive_name.text() { + fed_two_subgraph_names.push(subgraph_definition.name.clone()); + } + } + } + } + } + } + + if let Some(specified_federation_version) = maybe_specified_federation_version { + // error if we detect an `@link` directive and the explicitly set `federation_version` to 1 + if specified_federation_version.is_fed_one() && !fed_two_subgraph_names.is_empty() { + let mut err = + RoverError::new(anyhow!("The 'federation_version' set in '{}' is invalid. The following subgraphs contain '@link' directives, which are only valid in Federation 2: {}", unresolved_supergraph_yaml, fed_two_subgraph_names.join(", "))); + err.set_suggestion(Suggestion::Adhoc(format!( + "Either remove the 'federation_version' entry from '{}', or set the value to '2'.", + unresolved_supergraph_yaml + ))); + return Err(err); + } + + // otherwise, set the version to what they set + resolved_supergraph_config.set_federation_version(specified_federation_version) + } else if fed_two_subgraph_names.is_empty() { + // if they did not specify a version and no subgraphs contain `@link` directives, use Federation 1 + resolved_supergraph_config.set_federation_version(FederationVersion::LatestFedOne) + } else { + // if they did not specify a version and at least one subgraph contains an `@link` directive, use Federation 2 + resolved_supergraph_config.set_federation_version(FederationVersion::LatestFedTwo) + } + Ok(resolved_supergraph_config) } diff --git a/src/utils/parsers.rs b/src/utils/parsers.rs index 5dc97ac93..b9296ab8b 100644 --- a/src/utils/parsers.rs +++ b/src/utils/parsers.rs @@ -2,7 +2,7 @@ use camino::{Utf8Path, Utf8PathBuf}; use crate::{anyhow, error::RoverError, Context, Result, Suggestion}; -use std::io::Read; +use std::{fmt, io::Read}; #[derive(Debug, PartialEq)] pub enum FileDescriptorType { @@ -59,6 +59,19 @@ impl FileDescriptorType { } } +impl fmt::Display for FileDescriptorType { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "{}", + match self { + Self::File(config_path) => config_path.as_str(), + Self::Stdin => "stdin", + } + ) + } +} + pub fn parse_file_descriptor(input: &str) -> Result { if input == "-" { Ok(FileDescriptorType::Stdin)