From b8cc69a827321b6eac7592dffa614b3cd4bdd610 Mon Sep 17 00:00:00 2001 From: Brian George Date: Fri, 8 Nov 2024 12:21:48 -0500 Subject: [PATCH 1/6] Install Router Binary --- .cargo/config.toml | 5 +- Cargo.lock | 11 ++ Cargo.toml | 1 + src/command/dev/next/router/binary.rs | 15 ++ src/command/dev/next/router/install.rs | 250 +++++++++++++++++++++++ src/command/dev/next/router/mod.rs | 2 + src/composition/runner/mod.rs | 4 +- src/composition/supergraph/binary.rs | 96 ++------- src/composition/supergraph/install.rs | 251 ++++++++++++++++++++++++ src/composition/supergraph/mod.rs | 1 + src/composition/supergraph/version.rs | 37 +++- src/composition/watchers/composition.rs | 15 +- src/utils/effect/install.rs | 28 +++ src/utils/effect/mod.rs | 1 + 14 files changed, 628 insertions(+), 89 deletions(-) create mode 100644 src/command/dev/next/router/binary.rs create mode 100644 src/command/dev/next/router/install.rs create mode 100644 src/composition/supergraph/install.rs create mode 100644 src/utils/effect/install.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index ecffa99a2..d85a669f4 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -2,6 +2,5 @@ xtask = "run --package xtask --" rover = "run --package rover --" install-rover = "run --release --package rover -- install --force" -test-next = "test --all-features" -build-next = "build --features composition-js,dev-next" - +test-next = "test --all-features -- --nocapture" +build-next = "build --features composition-js,dev-next" \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 2dc23e342..b0a5c3e92 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4731,6 +4731,7 @@ dependencies = [ "strum_macros", "tap", "tar", + "temp-env", "tempfile", "termimad", "thiserror", @@ -5719,6 +5720,16 @@ dependencies = [ "xattr", ] +[[package]] +name = "temp-env" +version = "0.3.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "96374855068f47402c3121c6eed88d29cb1de8f3ab27090e273e420bdabcf050" +dependencies = [ + "futures", + "parking_lot", +] + [[package]] name = "tempfile" version = "3.12.0" diff --git a/Cargo.toml b/Cargo.toml index 58c1ea579..492d92f3f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -244,3 +244,4 @@ rstest = { workspace = true } serial_test = { workspace = true } speculoos = { workspace = true } tracing-test = "0.2.5" +temp-env = { version = "0.3.6", features = ["async_closure"] } diff --git a/src/command/dev/next/router/binary.rs b/src/command/dev/next/router/binary.rs new file mode 100644 index 000000000..78903faaa --- /dev/null +++ b/src/command/dev/next/router/binary.rs @@ -0,0 +1,15 @@ +use camino::Utf8PathBuf; +use semver::Version; + +#[derive(Clone, Debug)] +#[cfg_attr(test, derive(derive_getters::Getters))] +pub struct RouterBinary { + exe: Utf8PathBuf, + version: Version, +} + +impl RouterBinary { + pub fn new(exe: Utf8PathBuf, version: Version) -> RouterBinary { + RouterBinary { exe, version } + } +} diff --git a/src/command/dev/next/router/install.rs b/src/command/dev/next/router/install.rs new file mode 100644 index 000000000..94775d6d3 --- /dev/null +++ b/src/command/dev/next/router/install.rs @@ -0,0 +1,250 @@ +use apollo_federation_types::config::RouterVersion; +use async_trait::async_trait; +use camino::{Utf8Path, Utf8PathBuf}; +use semver::Version; + +use crate::{ + command::{install::Plugin, Install}, + options::LicenseAccepter, + utils::{client::StudioClientConfig, effect::install::InstallBinary}, +}; + +use super::binary::RouterBinary; + +#[derive(thiserror::Error, Debug)] +#[error("Failed to install the router")] +pub enum InstallRouterError { + #[error("unable to find dependency: \"{err}\"")] + MissingDependency { + /// The error while attempting to find the dependency + err: String, + }, + #[error("Missing filename for path: {path}")] + MissingFilename { path: Utf8PathBuf }, + #[error("Invalid semver version: \"{input}\"")] + Semver { + input: String, + source: semver::Error, + }, +} + +pub struct InstallRouter { + studio_client_config: StudioClientConfig, + router_version: RouterVersion, +} + +impl InstallRouter { + pub fn new( + router_version: RouterVersion, + studio_client_config: StudioClientConfig, + ) -> InstallRouter { + InstallRouter { + router_version, + studio_client_config, + } + } +} + +#[async_trait] +impl InstallBinary for InstallRouter { + type Binary = RouterBinary; + type Error = InstallRouterError; + async fn install( + &self, + override_install_path: Option, + elv2_license_accepter: LicenseAccepter, + skip_update: bool, + ) -> Result { + let plugin = Plugin::Router(self.router_version.clone()); + let install_command = Install { + force: false, + plugin: Some(plugin), + elv2_license_accepter, + }; + let exe = install_command + .get_versioned_plugin( + override_install_path, + self.studio_client_config.clone(), + skip_update, + ) + .await + .map_err(|err| InstallRouterError::MissingDependency { + err: err.to_string(), + })?; + let version = version_from_path(&exe)?; + let binary = RouterBinary::new(exe, version); + Ok(binary) + } +} + +fn version_from_path(path: &Utf8Path) -> Result { + let file_name = path + .file_name() + .ok_or_else(|| InstallRouterError::MissingFilename { + path: path.to_path_buf(), + })?; + let without_exe = file_name.strip_suffix(".exe").unwrap_or(file_name); + let without_prefix = without_exe.strip_prefix("router-v").unwrap_or(without_exe); + let version = Version::parse(without_prefix).map_err(|err| InstallRouterError::Semver { + input: without_prefix.to_string(), + source: err, + })?; + Ok(version) +} + +#[cfg(test)] +mod tests { + use std::{str::FromStr, time::Duration}; + + use anyhow::Result; + use apollo_federation_types::config::RouterVersion; + use assert_fs::{NamedTempFile, TempDir}; + use camino::Utf8PathBuf; + use flate2::{write::GzEncoder, Compression}; + use houston::Config; + use http::Method; + use httpmock::MockServer; + use rstest::{fixture, rstest}; + use semver::Version; + use speculoos::prelude::*; + use tracing_test::traced_test; + + use crate::{ + options::LicenseAccepter, + utils::{ + client::{ClientBuilder, StudioClientConfig}, + effect::install::InstallBinary, + }, + }; + + use super::InstallRouter; + + #[fixture] + #[once] + fn http_server() -> MockServer { + MockServer::start() + } + + #[fixture] + #[once] + fn mock_server_endpoint(http_server: &MockServer) -> String { + let address = http_server.address(); + let endpoint = format!("http://{}", address); + endpoint + } + + #[fixture] + #[once] + fn home() -> TempDir { + TempDir::new().unwrap() + } + + #[fixture] + fn router_version() -> RouterVersion { + RouterVersion::Latest + } + + #[fixture] + #[once] + fn api_key() -> String { + "api-key".to_string() + } + + #[fixture] + fn config(api_key: &str, home: &TempDir) -> Config { + let home = Utf8PathBuf::from_path_buf(home.to_path_buf()).unwrap(); + Config { + home, + override_api_key: Some(api_key.to_string()), + } + } + + #[fixture] + fn studio_client_config(mock_server_endpoint: &str, config: Config) -> StudioClientConfig { + StudioClientConfig::new( + Some(mock_server_endpoint.to_string()), + config, + false, + ClientBuilder::default(), + None, + ) + } + + #[traced_test] + #[tokio::test] + #[rstest] + #[timeout(Duration::from_secs(15))] + async fn test_install( + router_version: RouterVersion, + studio_client_config: StudioClientConfig, + http_server: &MockServer, + mock_server_endpoint: &str, + ) -> Result<()> { + let license_accepter = LicenseAccepter { + elv2_license_accepted: Some(true), + }; + let override_install_path = NamedTempFile::new("override_path")?; + let install_router = InstallRouter::new(router_version, studio_client_config); + http_server.mock(|when, then| { + when.matches(|request| { + request.method == Method::HEAD.to_string() + && request.path.starts_with("/tar/router") + }); + then.status(302).header("X-Version", "v1.57.1"); + }); + http_server.mock(|when, then| { + when.matches(|request| { + request.method == Method::GET.to_string() + && request.path.starts_with("/tar/router/") + }); + then.status(302) + .header("Location", format!("{}/router/", mock_server_endpoint)); + }); + + let enc = GzEncoder::new(Vec::new(), Compression::default()); + let mut archive = tar::Builder::new(enc); + let contents = b"router"; + let mut header = tar::Header::new_gnu(); + header.set_path("dist/router")?; + header.set_size(contents.len().try_into().unwrap()); + header.set_cksum(); + archive.append(&header, &contents[..]).unwrap(); + + let finished_archive = archive.into_inner()?; + let finished_archive_bytes = finished_archive.finish()?; + + http_server.mock(|when, then| { + when.matches(|request| { + request.method == Method::GET.to_string() && request.path.starts_with("/router") + }); + then.status(200) + .header("Content-Type", "application/octet-stream") + .body(&finished_archive_bytes); + }); + let binary = temp_env::async_with_vars( + [("APOLLO_ROVER_DOWNLOAD_HOST", Some(mock_server_endpoint))], + async { + install_router + .install( + Utf8PathBuf::from_path_buf(override_install_path.to_path_buf()).ok(), + license_accepter, + false, + ) + .await + }, + ) + .await; + let subject = assert_that!(binary).is_ok().subject; + assert_that!(subject.version()).is_equal_to(&Version::from_str("1.57.1")?); + + let installed_binary_path = override_install_path + .path() + .join(".rover/bin/router-v1.57.1"); + assert_that!(subject.exe()) + .is_equal_to(&Utf8PathBuf::from_path_buf(installed_binary_path.clone()).unwrap()); + assert_that!(installed_binary_path.exists()).is_equal_to(true); + let installed_binary_contents = std::fs::read(installed_binary_path)?; + assert_that!(installed_binary_contents).is_equal_to(b"router".to_vec()); + Ok(()) + } +} diff --git a/src/command/dev/next/router/mod.rs b/src/command/dev/next/router/mod.rs index ef68c3694..a73efccee 100644 --- a/src/command/dev/next/router/mod.rs +++ b/src/command/dev/next/router/mod.rs @@ -1 +1,3 @@ +pub mod binary; pub mod config; +pub mod install; diff --git a/src/composition/runner/mod.rs b/src/composition/runner/mod.rs index a237d25ee..d805980ba 100644 --- a/src/composition/runner/mod.rs +++ b/src/composition/runner/mod.rs @@ -25,7 +25,7 @@ use self::state::SetupSubgraphWatchers; use super::{ events::CompositionEvent, supergraph::{ - binary::SupergraphBinary, + binary::{OutputTarget, SupergraphBinary}, config::resolve::{ subgraph::LazilyResolvedSubgraph, FullyResolvedSubgraphs, LazilyResolvedSupergraphConfig, @@ -117,6 +117,7 @@ impl Runner { exec_command: ExecC, read_file: ReadF, write_file: WriteF, + output_target: OutputTarget, temp_dir: Utf8PathBuf, ) -> Runner> where @@ -131,6 +132,7 @@ impl Runner { .exec_command(exec_command) .read_file(read_file) .write_file(write_file) + .output_target(output_target) .temp_dir(temp_dir) .build(); Runner { diff --git a/src/composition/supergraph/binary.rs b/src/composition/supergraph/binary.rs index 457418353..e411aa4ff 100644 --- a/src/composition/supergraph/binary.rs +++ b/src/composition/supergraph/binary.rs @@ -4,79 +4,17 @@ use apollo_federation_types::{ config::FederationVersion, rover::{BuildErrors, BuildOutput, BuildResult}, }; -use async_trait::async_trait; use buildstructor::Builder; use camino::Utf8PathBuf; -use rover_std::RoverStdError; use tap::TapFallible; use crate::{ - command::{install::Plugin, Install}, composition::{CompositionError, CompositionSuccess}, - options::LicenseAccepter, - utils::{ - client::StudioClientConfig, - effect::{exec::ExecCommand, read_file::ReadFile}, - }, + utils::effect::{exec::ExecCommand, read_file::ReadFile}, }; use super::version::SupergraphVersion; -/// This trait allows us to mock the installation of the supergraph binary -#[cfg_attr(test, mockall::automock(type Error = MockInstallSupergraphBinaryError;))] -#[async_trait] -pub trait InstallSupergraphBinary { - type Error: std::error::Error + Send + 'static; - - async fn install(&self) -> Result; -} - -#[async_trait] -impl InstallSupergraphBinary for InstallSupergraph { - type Error = RoverStdError; - - async fn install(&self) -> Result { - if self.federation_version.is_fed_two() { - self.elv2_license_accepter - .require_elv2_license(&self.studio_client_config) - .map_err(|_err| RoverStdError::LicenseNotAccepted)? - } - - let plugin = Plugin::Supergraph(self.federation_version.clone()); - - let install_command = Install { - force: false, - plugin: Some(plugin), - elv2_license_accepter: self.elv2_license_accepter, - }; - - let exe = install_command - .get_versioned_plugin( - self.override_install_path.clone(), - self.studio_client_config.clone(), - self.skip_update, - ) - .await - .map_err(|err| RoverStdError::MissingDependency { - err: err.to_string(), - })?; - - Ok(exe) - } -} - -/// The installer for the supergraph binary. It implements InstallSupergraphBinary and has an -/// `install()` method for the actual installation. Use the installed binary path when building the -/// SupergraphBinary struct -#[derive(Builder)] -pub struct InstallSupergraph { - federation_version: FederationVersion, - elv2_license_accepter: LicenseAccepter, - studio_client_config: StudioClientConfig, - override_install_path: Option, - skip_update: bool, -} - #[derive(Clone, Debug, Eq, PartialEq)] pub enum OutputTarget { File(Utf8PathBuf), @@ -118,22 +56,22 @@ impl From for CompositionError { } } -#[cfg_attr(test, derive(thiserror::Error, Debug))] -#[cfg_attr(test, error("MockInstallSupergraphBinaryError"))] -pub struct MockInstallSupergraphBinaryError {} - #[derive(Builder, Debug, Clone)] +#[cfg_attr(test, derive(derive_getters::Getters))] pub struct SupergraphBinary { exe: Utf8PathBuf, - output_target: OutputTarget, version: SupergraphVersion, } impl SupergraphBinary { - fn prepare_compose_args(&self, supergraph_config_path: &Utf8PathBuf) -> Vec { + fn prepare_compose_args( + &self, + output_target: &OutputTarget, + supergraph_config_path: &Utf8PathBuf, + ) -> Vec { let mut args = vec!["compose".to_string(), supergraph_config_path.to_string()]; - if let OutputTarget::File(output_path) = &self.output_target { + if let OutputTarget::File(output_path) = output_target { args.push(output_path.to_string()); } @@ -144,9 +82,11 @@ impl SupergraphBinary { &self, exec_impl: &impl ExecCommand, read_file_impl: &impl ReadFile, + output_target: &OutputTarget, supergraph_config_path: Utf8PathBuf, ) -> Result { - let args = self.prepare_compose_args(&supergraph_config_path); + let args = self.prepare_compose_args(output_target, &supergraph_config_path); + let output = exec_impl .exec_command(&self.exe, &args) .await @@ -155,7 +95,7 @@ impl SupergraphBinary { error: format!("{:?}", err), })?; - let output = match &self.output_target { + let output = match output_target { OutputTarget::File(path) => { read_file_impl .read_file(path) @@ -338,11 +278,11 @@ mod tests { let supergraph_binary = SupergraphBinary::builder() .exe(Utf8PathBuf::from_str("some/binary").unwrap()) - .output_target(test_output_target) .version(supergraph_version) .build(); - let args = supergraph_binary.prepare_compose_args(&supergraph_config_path); + let args = + supergraph_binary.prepare_compose_args(&test_output_target, &supergraph_config_path); assert_eq!(args, expected_args); } @@ -358,7 +298,6 @@ mod tests { let supergraph_binary = SupergraphBinary::builder() .exe(binary_path.clone()) - .output_target(OutputTarget::Stdout) .version(supergraph_version) .build(); @@ -388,7 +327,12 @@ mod tests { }); let result = supergraph_binary - .compose(&mock_exec, &mock_read_file, temp_supergraph_config_path) + .compose( + &mock_exec, + &mock_read_file, + &OutputTarget::Stdout, + temp_supergraph_config_path, + ) .await; assert_that!(result).is_ok().is_equal_to(composition_output); diff --git a/src/composition/supergraph/install.rs b/src/composition/supergraph/install.rs new file mode 100644 index 000000000..118879eda --- /dev/null +++ b/src/composition/supergraph/install.rs @@ -0,0 +1,251 @@ +use apollo_federation_types::config::FederationVersion; +use async_trait::async_trait; +use camino::Utf8PathBuf; + +use crate::{ + command::{install::Plugin, Install}, + options::LicenseAccepter, + utils::{client::StudioClientConfig, effect::install::InstallBinary}, +}; + +use super::{ + binary::SupergraphBinary, + version::{SupergraphVersion, SupergraphVersionError}, +}; + +#[derive(thiserror::Error, Debug)] +pub enum InstallSupergraphError { + #[error("ELV2 license must be accepted")] + LicenseNotAccepted, + #[error("unable to find dependency: \"{err}\"")] + MissingDependency { + /// The error while attempting to find the dependency + err: String, + }, + #[error(transparent)] + SupergraphVersion(#[from] SupergraphVersionError), +} + +/// The installer for the supergraph binary. It implements InstallSupergraphBinary and has an +/// `install()` method for the actual installation. Use the installed binary path when building the +/// SupergraphBinary struct +pub struct InstallSupergraph { + federation_version: FederationVersion, + studio_client_config: StudioClientConfig, +} + +impl InstallSupergraph { + pub fn new( + federation_version: FederationVersion, + studio_client_config: StudioClientConfig, + ) -> InstallSupergraph { + InstallSupergraph { + federation_version, + studio_client_config, + } + } +} + +#[async_trait] +impl InstallBinary for InstallSupergraph { + type Binary = SupergraphBinary; + type Error = InstallSupergraphError; + + async fn install( + &self, + override_install_path: Option, + elv2_license_accepter: LicenseAccepter, + skip_update: bool, + ) -> Result { + if self.federation_version.is_fed_two() { + elv2_license_accepter + .require_elv2_license(&self.studio_client_config) + .map_err(|_err| InstallSupergraphError::LicenseNotAccepted)? + } + + let plugin = Plugin::Supergraph(self.federation_version.clone()); + + let install_command = Install { + force: false, + plugin: Some(plugin), + elv2_license_accepter, + }; + + let exe = install_command + .get_versioned_plugin( + override_install_path, + self.studio_client_config.clone(), + skip_update, + ) + .await + .map_err(|err| InstallSupergraphError::MissingDependency { + err: err.to_string(), + })?; + + let version = SupergraphVersion::try_from(&exe)?; + let binary = SupergraphBinary::builder() + .exe(exe) + .version(version) + .build(); + + Ok(binary) + } +} + +#[cfg(test)] +mod tests { + use std::{str::FromStr, time::Duration}; + + use anyhow::Result; + use apollo_federation_types::config::FederationVersion; + use assert_fs::{NamedTempFile, TempDir}; + use camino::Utf8PathBuf; + use flate2::{write::GzEncoder, Compression}; + use houston::Config; + use httpmock::{Method, MockServer}; + use rstest::{fixture, rstest}; + use semver::Version; + use speculoos::prelude::*; + use tracing_test::traced_test; + + use crate::{ + composition::supergraph::version::SupergraphVersion, + options::LicenseAccepter, + utils::{ + client::{ClientBuilder, StudioClientConfig}, + effect::install::InstallBinary, + }, + }; + + use super::InstallSupergraph; + + #[fixture] + #[once] + fn http_server() -> MockServer { + MockServer::start() + } + + #[fixture] + #[once] + fn mock_server_endpoint(http_server: &MockServer) -> String { + let address = http_server.address(); + let endpoint = format!("http://{}", address); + endpoint + } + + #[fixture] + #[once] + fn home() -> TempDir { + TempDir::new().unwrap() + } + + #[fixture] + fn federation_version() -> FederationVersion { + FederationVersion::LatestFedTwo + } + + #[fixture] + #[once] + fn api_key() -> String { + "api-key".to_string() + } + + #[fixture] + fn config(api_key: &str, home: &TempDir) -> Config { + let home = Utf8PathBuf::from_path_buf(home.to_path_buf()).unwrap(); + Config { + home, + override_api_key: Some(api_key.to_string()), + } + } + + #[fixture] + fn studio_client_config(mock_server_endpoint: &str, config: Config) -> StudioClientConfig { + StudioClientConfig::new( + Some(mock_server_endpoint.to_string()), + config, + false, + ClientBuilder::default(), + None, + ) + } + + #[traced_test] + #[tokio::test] + #[rstest] + #[timeout(Duration::from_secs(15))] + async fn test_install( + federation_version: FederationVersion, + studio_client_config: StudioClientConfig, + http_server: &MockServer, + mock_server_endpoint: &str, + ) -> Result<()> { + let license_accepter = LicenseAccepter { + elv2_license_accepted: Some(true), + }; + let override_install_path = NamedTempFile::new("override_path")?; + let install_supergraph = InstallSupergraph::new(federation_version, studio_client_config); + http_server.mock(|when, then| { + when.matches(|request| { + request.method == Method::HEAD.to_string() + && request.path.starts_with("/tar/supergraph") + }); + then.status(302).header("X-Version", "v2.9.0"); + }); + http_server.mock(|when, then| { + when.matches(|request| { + request.method == Method::GET.to_string() + && request.path.starts_with("/tar/supergraph/") + }); + then.status(302) + .header("Location", format!("{}/supergraph/", mock_server_endpoint)); + }); + + let enc = GzEncoder::new(Vec::new(), Compression::default()); + let mut archive = tar::Builder::new(enc); + let contents = b"supergraph"; + let mut header = tar::Header::new_gnu(); + header.set_path("dist/supergraph")?; + header.set_size(contents.len().try_into().unwrap()); + header.set_cksum(); + archive.append(&header, &contents[..]).unwrap(); + + let finished_archive = archive.into_inner()?; + let finished_archive_bytes = finished_archive.finish()?; + + http_server.mock(|when, then| { + when.matches(|request| { + request.method == Method::GET.to_string() && request.path.starts_with("/supergraph") + }); + then.status(200) + .header("Content-Type", "application/octet-stream") + .body(&finished_archive_bytes); + }); + let binary = temp_env::async_with_vars( + [("APOLLO_ROVER_DOWNLOAD_HOST", Some(mock_server_endpoint))], + async { + install_supergraph + .install( + Utf8PathBuf::from_path_buf(override_install_path.to_path_buf()).ok(), + license_accepter, + false, + ) + .await + }, + ) + .await; + let subject = assert_that!(binary).is_ok().subject; + assert_that!(subject.version()) + .is_equal_to(&SupergraphVersion::new(Version::from_str("2.9.0")?)); + + let installed_binary_path = override_install_path + .path() + .join(".rover/bin/supergraph-v2.9.0"); + assert_that!(subject.exe()) + .is_equal_to(&Utf8PathBuf::from_path_buf(installed_binary_path.clone()).unwrap()); + assert_that!(installed_binary_path.exists()).is_equal_to(true); + let installed_binary_contents = std::fs::read(installed_binary_path)?; + assert_that!(installed_binary_contents).is_equal_to(b"supergraph".to_vec()); + Ok(()) + } +} diff --git a/src/composition/supergraph/mod.rs b/src/composition/supergraph/mod.rs index fdbb66554..09d4c0c97 100644 --- a/src/composition/supergraph/mod.rs +++ b/src/composition/supergraph/mod.rs @@ -1,3 +1,4 @@ pub mod binary; pub mod config; +pub mod install; pub mod version; diff --git a/src/composition/supergraph/version.rs b/src/composition/supergraph/version.rs index 005efcaf1..48a3e3cdc 100644 --- a/src/composition/supergraph/version.rs +++ b/src/composition/supergraph/version.rs @@ -1,15 +1,23 @@ use std::{fmt::Display, str::FromStr}; use apollo_federation_types::config::FederationVersion; +use camino::Utf8PathBuf; use semver::Version; use serde_json::Value; -#[derive(thiserror::Error, Debug, PartialEq)] +#[derive(thiserror::Error, Debug)] pub enum SupergraphVersionError { #[error("Unsupported Federation version: {}", .version.to_string())] UnsupportedFederationVersion { version: SupergraphVersion }, #[error("Unable to get version: {}", .error)] Conversion { error: String }, + #[error("Filename does not exist at the given path")] + MissingFilename, + #[error("Semver could not be extracted from the installed path")] + InvalidVersion { + #[from] + source: semver::Error, + }, } #[derive(Debug, Clone, Eq, PartialEq)] @@ -33,6 +41,22 @@ impl Display for SupergraphVersion { } } +impl TryFrom<&Utf8PathBuf> for SupergraphVersion { + type Error = SupergraphVersionError; + fn try_from(value: &Utf8PathBuf) -> Result { + let file_name = value + .file_name() + .ok_or_else(|| SupergraphVersionError::MissingFilename)?; + let without_exe = file_name.strip_suffix(".exe").unwrap_or(file_name); + let version = Version::parse( + without_exe + .strip_prefix("supergraph-v") + .unwrap_or(without_exe), + )?; + Ok(SupergraphVersion { version }) + } +} + impl TryFrom for FederationVersion { type Error = SupergraphVersionError; fn try_from(supergraph_version: SupergraphVersion) -> Result { @@ -238,11 +262,12 @@ mod tests { } else { let conversion: Result = supergraph_version.clone().try_into(); - assert_that!(conversion).is_err_containing( - SupergraphVersionError::UnsupportedFederationVersion { - version: supergraph_version, - }, - ) + assert_that!(conversion).is_err().matches(|err| match err { + SupergraphVersionError::UnsupportedFederationVersion { version } => { + version == &supergraph_version + } + _ => false, + }); } } } diff --git a/src/composition/watchers/composition.rs b/src/composition/watchers/composition.rs index bb7d22de3..8e93461d4 100644 --- a/src/composition/watchers/composition.rs +++ b/src/composition/watchers/composition.rs @@ -10,7 +10,10 @@ use tokio_stream::StreamExt; use crate::{ composition::{ events::CompositionEvent, - supergraph::{binary::SupergraphBinary, config::resolve::FullyResolvedSubgraphs}, + supergraph::{ + binary::{OutputTarget, SupergraphBinary}, + config::resolve::FullyResolvedSubgraphs, + }, watchers::{subgraphs::SubgraphEvent, subtask::SubtaskHandleStream}, }, utils::effect::{exec::ExecCommand, read_file::ReadFile, write_file::WriteFile}, @@ -20,6 +23,7 @@ use crate::{ pub struct CompositionWatcher { subgraphs: FullyResolvedSubgraphs, supergraph_binary: SupergraphBinary, + output_target: OutputTarget, exec_command: ExecC, read_file: ReadF, write_file: WriteF, @@ -86,7 +90,12 @@ where let output = self .supergraph_binary - .compose(&self.exec_command, &self.read_file, target_file.clone()) + .compose( + &self.exec_command, + &self.read_file, + &self.output_target, + target_file.clone(), + ) .await; match output { @@ -166,7 +175,6 @@ mod tests { let supergraph_version = SupergraphVersion::new(Version::from_str("2.8.0").unwrap()); let supergraph_binary = SupergraphBinary::builder() - .output_target(OutputTarget::Stdout) .version(supergraph_version) .exe(Utf8PathBuf::from_str("some/binary").unwrap()) .build(); @@ -220,6 +228,7 @@ mod tests { .read_file(mock_read_file) .write_file(mock_write_file) .temp_dir(temp_dir_path) + .output_target(OutputTarget::Stdout) .build(); let subgraph_change_events: BoxStream = once(async { diff --git a/src/utils/effect/install.rs b/src/utils/effect/install.rs new file mode 100644 index 000000000..32b472629 --- /dev/null +++ b/src/utils/effect/install.rs @@ -0,0 +1,28 @@ +use async_trait::async_trait; +use camino::Utf8PathBuf; + +use crate::options::LicenseAccepter; + +#[cfg_attr(test, derive(thiserror::Error, Debug))] +#[cfg_attr(test, error("MockInstallBinaryError"))] +pub struct MockInstallBinaryError {} + +#[cfg(test)] +pub struct MockBinary {} + +#[cfg_attr(test, mockall::automock( + type Binary = MockBinary; + type Error = MockInstallBinaryError; +))] +#[async_trait] +pub trait InstallBinary { + type Binary; + type Error: std::error::Error + Send + 'static; + + async fn install( + &self, + override_install_path: Option, + elv2_license_accepter: LicenseAccepter, + skip_update: bool, + ) -> Result; +} diff --git a/src/utils/effect/mod.rs b/src/utils/effect/mod.rs index 1d1ec78a8..7a6f05569 100644 --- a/src/utils/effect/mod.rs +++ b/src/utils/effect/mod.rs @@ -1,6 +1,7 @@ pub mod exec; pub mod fetch_remote_subgraph; pub mod fetch_remote_subgraphs; +pub mod install; pub mod introspect; pub mod read_file; pub mod read_stdin; From 39414d12e06ccd1a898f9f1de5ead03aa834575a Mon Sep 17 00:00:00 2001 From: Brian George Date: Fri, 8 Nov 2024 16:34:23 -0500 Subject: [PATCH 2/6] Update --- src/command/dev/next/router/binary.rs | 2 + src/command/dev/next/router/install.rs | 1 + src/command/supergraph/compose/do_compose.rs | 40 +++++++++----------- src/composition/supergraph/version.rs | 14 ++++--- 4 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/command/dev/next/router/binary.rs b/src/command/dev/next/router/binary.rs index 78903faaa..ad6ca4678 100644 --- a/src/command/dev/next/router/binary.rs +++ b/src/command/dev/next/router/binary.rs @@ -4,7 +4,9 @@ use semver::Version; #[derive(Clone, Debug)] #[cfg_attr(test, derive(derive_getters::Getters))] pub struct RouterBinary { + #[allow(unused)] exe: Utf8PathBuf, + #[allow(unused)] version: Version, } diff --git a/src/command/dev/next/router/install.rs b/src/command/dev/next/router/install.rs index 94775d6d3..b92951a20 100644 --- a/src/command/dev/next/router/install.rs +++ b/src/command/dev/next/router/install.rs @@ -34,6 +34,7 @@ pub struct InstallRouter { } impl InstallRouter { + #[allow(unused)] pub fn new( router_version: RouterVersion, studio_client_config: StudioClientConfig, diff --git a/src/command/supergraph/compose/do_compose.rs b/src/command/supergraph/compose/do_compose.rs index b7c014c8b..41aa584a3 100644 --- a/src/command/supergraph/compose/do_compose.rs +++ b/src/command/supergraph/compose/do_compose.rs @@ -40,7 +40,7 @@ use crate::{ events::CompositionEvent, runner::Runner, supergraph::{ - binary::{InstallSupergraph, InstallSupergraphBinary, OutputTarget, SupergraphBinary}, + binary::{OutputTarget, SupergraphBinary}, config::{ resolve::{ subgraph::FullyResolvedSubgraph, FullyResolvedSubgraphs, @@ -48,6 +48,7 @@ use crate::{ }, SupergraphConfigResolver, }, + install::InstallSupergraph, version::SupergraphVersion, }, }, @@ -57,6 +58,7 @@ use crate::{ effect::{ exec::TokioCommand, fetch_remote_subgraph::RemoteSubgraph, + install::InstallBinary, read_file::FsReadFile, write_file::{FsWriteFile, WriteFile}, }, @@ -260,31 +262,23 @@ impl Compose { } // Build the supergraph binary, paying special attention to the CLI options - let supergraph_binary = InstallSupergraph::builder() - .federation_version(fed_version.clone()) - .elv2_license_accepter(self.opts.plugin_opts.elv2_license_accepter) - .studio_client_config(client_config.clone()) - .and_override_install_path(override_install_path) - .skip_update(self.opts.plugin_opts.skip_update) - .build() - .install() + let supergraph_binary = InstallSupergraph::new(fed_version.clone(), client_config.clone()) + .install( + override_install_path, + self.opts.plugin_opts.elv2_license_accepter, + self.opts.plugin_opts.skip_update, + ) .await?; - // Federation versions and supergraph versions are synonymous, but have different types - // representing them depending on their use (eg, we only ever have an exact - // SupergraphVersion because we can only ever use an exact version of the binary, where the - // FederationVersion can just point to latest to get the latest version--these are similar, - // but represent different ways of using the underlying version) - let supergraph_version: SupergraphVersion = fed_version.clone().try_into()?; - - let supergraph_binary = SupergraphBinary::builder() - .exe(supergraph_binary) - .output_target(output_file) - .version(supergraph_version) - .build(); - let result = supergraph_binary - .compose(&exec_command, &read_file, supergraph_config_filepath) + .compose( + &exec_command, + &read_file, + &output_file + .map(|path| OutputTarget::File(path)) + .unwrap_or_else(|| OutputTarget::Stdout), + supergraph_config_filepath, + ) .await?; Ok(RoverOutput::CompositionResult(result.into())) diff --git a/src/composition/supergraph/version.rs b/src/composition/supergraph/version.rs index 48a3e3cdc..0346adf74 100644 --- a/src/composition/supergraph/version.rs +++ b/src/composition/supergraph/version.rs @@ -213,26 +213,28 @@ mod tests { #[rstest] #[case::exact_fed_one( FederationVersion::ExactFedOne(fed_one()), - Ok(SupergraphVersion::new(fed_one())) + SupergraphVersion::new(fed_one()) )] #[case::exact_fed_two( FederationVersion::ExactFedTwo(fed_two_eight()), - Ok(SupergraphVersion::new(fed_two_eight())) + SupergraphVersion::new(fed_two_eight()) )] #[case::latest_fed_one( FederationVersion::LatestFedOne, - Ok(SupergraphVersion::new(latest_fed_one())) + SupergraphVersion::new(latest_fed_one()) )] #[case::latest_fed_two( FederationVersion::LatestFedTwo, - Ok(SupergraphVersion::new(latest_fed_two())) + SupergraphVersion::new(latest_fed_two()) )] fn test_tryfrom_fedversion_for_supergraphversion( #[case] fed_version: FederationVersion, - #[case] expected: Result, + #[case] expected: SupergraphVersion, ) { let supergraph_version = TryInto::::try_into(fed_version); - assert_that!(supergraph_version).is_equal_to(expected) + assert_that!(supergraph_version) + .is_ok() + .is_equal_to(expected) } #[rstest] From 9e3811f0953f21fcfb19cd52dc103ed92dd25d11 Mon Sep 17 00:00:00 2001 From: Brian George Date: Wed, 20 Nov 2024 10:32:17 -0500 Subject: [PATCH 3/6] Fix linter warning --- src/composition/runner/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/composition/runner/mod.rs b/src/composition/runner/mod.rs index d805980ba..71fd440bb 100644 --- a/src/composition/runner/mod.rs +++ b/src/composition/runner/mod.rs @@ -110,6 +110,7 @@ impl Runner { impl Runner { /// Configures the composition watcher + #[allow(clippy::too_many_arguments)] pub fn setup_composition_watcher( self, subgraphs: FullyResolvedSubgraphs, From e28bc3e7b5fa24139c6fe60d3197ee4305267073 Mon Sep 17 00:00:00 2001 From: Brian George Date: Wed, 20 Nov 2024 11:11:14 -0500 Subject: [PATCH 4/6] Update test tarball path for windows --- src/composition/supergraph/install.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/composition/supergraph/install.rs b/src/composition/supergraph/install.rs index 118879eda..3ff8052b2 100644 --- a/src/composition/supergraph/install.rs +++ b/src/composition/supergraph/install.rs @@ -205,7 +205,11 @@ mod tests { let mut archive = tar::Builder::new(enc); let contents = b"supergraph"; let mut header = tar::Header::new_gnu(); - header.set_path("dist/supergraph")?; + if cfg!(windows) { + header.set_path("dist/supergraph.exe")?; + } else { + header.set_path("dist/supergraph")?; + } header.set_size(contents.len().try_into().unwrap()); header.set_cksum(); archive.append(&header, &contents[..]).unwrap(); From 7639dcd2f3b6fb910a51d0b61e4d692c0338da2c Mon Sep 17 00:00:00 2001 From: Brian George Date: Wed, 20 Nov 2024 11:28:51 -0500 Subject: [PATCH 5/6] Update windows path expectation --- src/composition/supergraph/install.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/composition/supergraph/install.rs b/src/composition/supergraph/install.rs index 3ff8052b2..d18ec4d22 100644 --- a/src/composition/supergraph/install.rs +++ b/src/composition/supergraph/install.rs @@ -242,9 +242,16 @@ mod tests { assert_that!(subject.version()) .is_equal_to(&SupergraphVersion::new(Version::from_str("2.9.0")?)); + let bin_name = if cfg!(windows) { + "supergraph-v2.9.0.exe" + } else { + "supergraph-v2.9.0" + }; + let installed_binary_path = override_install_path .path() - .join(".rover/bin/supergraph-v2.9.0"); + .join(".rover/bin") + .join(bin_name); assert_that!(subject.exe()) .is_equal_to(&Utf8PathBuf::from_path_buf(installed_binary_path.clone()).unwrap()); assert_that!(installed_binary_path.exists()).is_equal_to(true); From d8207c70e639b0e59db1062a7cf7e697581c056c Mon Sep 17 00:00:00 2001 From: Brian George Date: Wed, 20 Nov 2024 12:35:14 -0500 Subject: [PATCH 6/6] Update docs --- src/composition/supergraph/install.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/composition/supergraph/install.rs b/src/composition/supergraph/install.rs index d18ec4d22..c79f9ab38 100644 --- a/src/composition/supergraph/install.rs +++ b/src/composition/supergraph/install.rs @@ -26,9 +26,9 @@ pub enum InstallSupergraphError { SupergraphVersion(#[from] SupergraphVersionError), } -/// The installer for the supergraph binary. It implements InstallSupergraphBinary and has an +/// The installer for the supergraph binary. It implements [`InstallSupergraph`] and has an /// `install()` method for the actual installation. Use the installed binary path when building the -/// SupergraphBinary struct +/// [`SupergraphBinary`] struct pub struct InstallSupergraph { federation_version: FederationVersion, studio_client_config: StudioClientConfig,