Bring dev launch to parity with legacy (#2319)

This PR implements the following requirements specified in the test plan

> **When I start rover dev with a supergraph config and --graph-ref**
> 
> If the subgraphs compose
> * I should see “starting a session with the ‘<subgraph name>’'
subgraph” for each subgraph in the config
> * I should see “composing supergraph with Federation < federation
version >” with the version of federation in the supergraph config
> * If `--federation-version` is specified, this is the value that
appears
> * If no federation version is specified, the latest Fed2 version
should be used
> * If a Fed1 version is specified with Fed2 subgraphs, composition
should fail specifying that this was the reason
>* If the supergraph does not compose
> * I should see an error explaining why the supergraph composition
failed

Have included screenshots below for comparison purposes

**Old dev launch**
![Screenshot 2024-12-20 at 11 13
43](https://github.com/user-attachments/assets/1c2e46e0-6d73-4760-80d6-af1d960c66da)

**New dev launch**
![Screenshot 2024-12-20 at 11 14
41](https://github.com/user-attachments/assets/c13eb8fc-569a-41af-ba1b-5f087b969715)

**Specifying a Federation Version**
![Screenshot 2024-12-20 at 11 15
49](https://github.com/user-attachments/assets/539e01c7-324a-431d-a1c5-86e7498a5a1c)

**Specifying a Fed 1 Version with Fed 2 Subgraphs**
![Screenshot 2024-12-20 at 11 17
08](https://github.com/user-attachments/assets/40a37813-029d-4195-a5ea-c0e6ab45431f)

**Supergraph doesn't compose**
![Screenshot 2024-12-20 at 11 17
51](https://github.com/user-attachments/assets/29836f06-2bea-4004-aa29-60029f7f9768)

Author: jonathanrainer

src/command/dev/next/mod.rs

@@ -3,13 +3,16 @@
 use std::io::stdin;
 
 use anyhow::anyhow;
+use apollo_federation_types::config::FederationVersion::LatestFedTwo;
 use apollo_federation_types::config::RouterVersion;
 use camino::Utf8PathBuf;
 use futures::StreamExt;
 use houston::{Config, Profile};
 use router::{install::InstallRouter, run::RunRouter, watchers::file::FileWatcher};
 use rover_client::operations::config::who_am_i::WhoAmI;
+use rover_std::{infoln, warnln};
 
+use self::router::config::{RouterAddress, RunRouterConfig};
 use crate::{
     command::Dev,
     composition::{
@@ -30,8 +33,6 @@ use crate::{
     RoverError, RoverOutput, RoverResult,
 };
 
-use self::router::config::{RouterAddress, RunRouterConfig};
-
 mod router;
 
 impl Dev {
@@ -65,6 +66,9 @@ impl Dev {
 
         let profile = &self.opts.plugin_opts.profile;
         let graph_ref = &self.opts.supergraph_opts.graph_ref;
+        if let Some(graph_ref) = graph_ref {
+            eprintln!("retrieving subgraphs remotely from {graph_ref}")
+        }
         let supergraph_config_path = &self.opts.supergraph_opts.clone().supergraph_config_path;
 
         let service = client_config.get_authenticated_client(profile)?.service()?;
@@ -90,7 +94,11 @@ impl Dev {
             .resolve_federation_version(
                 &client_config,
                 make_fetch_remote_subgraph,
-                self.opts.supergraph_opts.federation_version.clone(),
+                self.opts
+                    .supergraph_opts
+                    .federation_version
+                    .clone()
+                    .or(Some(LatestFedTwo)),
             )
             .await?
             .install_supergraph_binary(
@@ -100,6 +108,7 @@ impl Dev {
                 skip_update,
             )
             .await?;
+
         let composition_success = composition_pipeline
             .compose(&exec_command_impl, &read_file_impl, &write_file_impl, None)
             .await?;
@@ -126,7 +135,12 @@ impl Dev {
 
         let composition_messages = composition_runner.run();
 
-        let mut run_router = RunRouter::default()
+        eprintln!(
+            "composing supergraph with Federation {}",
+            composition_pipeline.state.supergraph_binary.version()
+        );
+
+        let run_router = RunRouter::default()
             .install::<InstallRouter>(
                 router_version,
                 client_config.clone(),
@@ -138,7 +152,9 @@ impl Dev {
             .load_config(&read_file_impl, router_address, router_config_path)
             .await?
             .load_remote_config(service, graph_ref.clone(), Some(credential))
-            .await
+            .await;
+        let router_address = run_router.state.config.address().clone();
+        let mut run_router = run_router
             .run(
                 FsWriteFile::default(),
                 TokioSpawn::default(),
@@ -150,6 +166,12 @@ impl Dev {
             .watch_for_changes(write_file_impl, composition_messages)
             .await;
 
+        warnln!(
+            "Do not run this command in production! It is intended for local development only."
+        );
+
+        infoln!("your supergraph is running! head to {router_address} to query your supergraph");
+
         loop {
             tokio::select! {
                 _ = tokio::signal::ctrl_c() => {
@@ -160,7 +182,9 @@ impl Dev {
                 Some(router_log) = run_router.router_logs().next() => {
                     match router_log {
                         Ok(router_log) => {
-                            eprintln!("{}", router_log);
+                            if !router_log.to_string().is_empty() {
+                        eprintln!("{}", router_log);
+                    }
                         }
                         Err(err) => {
                             tracing::error!("{:?}", err);
@@ -170,7 +194,6 @@ impl Dev {
                 else => break,
             }
         }
-
         Ok(RoverOutput::EmptySuccess)
     }
 }

src/command/dev/next/router/config/mod.rs

@@ -1,3 +1,4 @@
+use std::fmt::{Display, Formatter};
 use std::net::{IpAddr, Ipv4Addr, SocketAddr};
 
 use buildstructor::buildstructor;
@@ -6,12 +7,11 @@ use http::Uri;
 use rover_std::{Fs, RoverStdError};
 use thiserror::Error;
 
-use crate::utils::effect::read_file::ReadFile;
-
 use self::{
     parser::{ParseRouterConfigError, RouterConfigParser},
     state::{RunRouterConfigDefault, RunRouterConfigFinal, RunRouterConfigReadConfig},
 };
+use crate::utils::effect::read_file::ReadFile;
 
 mod parser;
 pub mod remote;
@@ -54,6 +54,19 @@ impl RouterAddress {
     }
 }
 
+impl Display for RouterAddress {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let host = self
+            .host
+            .to_string()
+            .replace("127.0.0.1", "localhost")
+            .replace("0.0.0.0", "localhost")
+            .replace("[::]", "localhost")
+            .replace("[::1]", "localhost");
+        write!(f, "http://{}:{}", host, self.port)
+    }
+}
+
 impl Default for RouterAddress {
     fn default() -> Self {
         RouterAddress {
@@ -178,6 +191,10 @@ impl RunRouterConfig<RunRouterConfigFinal> {
         &self.state.health_check_endpoint
     }
 
+    pub fn raw_config(&self) -> String {
+        self.state.raw_config.clone()
+    }
+
     #[allow(unused)]
     pub fn router_config(&self) -> RouterConfig {
         RouterConfig(self.state.raw_config.to_string())

src/command/dev/next/router/config/parser.rs

@@ -84,7 +84,7 @@ impl<'a> RouterConfigParser<'a> {
             .and_then(|addr_and_port| Some(Uri::from_str(addr_and_port)))
             // See https://www.apollographql.com/docs/graphos/routing/self-hosted/health-checks for
             // defaults
-            .unwrap_or(Uri::from_str("127.0.0.1:8088"))
+            .unwrap_or(Uri::from_str("http://127.0.0.1:8088"))
             .map_err(|err| ParseRouterConfigError::ListenPath {
                 path: "health_check.listen",
                 source: err,
@@ -161,8 +161,8 @@ health_check:
         );
         let config_yaml = serde_yaml::from_str(&config_yaml_str)?;
         let router_config = RouterConfigParser { yaml: &config_yaml };
-        let health_check = router_config.health_check();
-        assert_that!(health_check).is_equal_to(is_health_check_enabled);
+        let health_check = router_config.health_check_endpoint();
+        assert_that!(health_check.is_ok()).is_equal_to(is_health_check_enabled);
         Ok(())
     }
 
@@ -174,9 +174,8 @@ health_check:
         };
         let config_yaml = serde_yaml::from_str(&config_yaml_str)?;
         let router_config = RouterConfigParser { yaml: &config_yaml };
-        let health_check = router_config.health_check();
-        assert_that!(health_check).is_false();
-        Ok(())
+        let health_check = router_config.health_check_endpoint();
+        assert_that!(health_check).is_equal_to(Ok(Uri::from_str("http://127.0.0.1:8088/health")?))
     }
 
     #[rstest]

src/command/dev/next/router/run.rs

@@ -11,12 +11,19 @@ use rover_client::{
     operations::config::who_am_i::{RegistryIdentity, WhoAmIError, WhoAmIRequest},
     shared::GraphRef,
 };
-use rover_std::RoverStdError;
+use rover_std::{infoln, RoverStdError};
 use tokio::{process::Child, time::sleep};
 use tokio_stream::wrappers::UnboundedReceiverStream;
 use tower::{Service, ServiceExt};
 use tracing::info;
 
+use super::{
+    binary::{RouterLog, RunRouterBinary, RunRouterBinaryError},
+    config::{remote::RemoteRouterConfig, ReadRouterConfigError, RouterAddress, RunRouterConfig},
+    hot_reload::{HotReloadEvent, HotReloadWatcher, RouterUpdateEvent},
+    install::{InstallRouter, InstallRouterError},
+    watchers::router_config::RouterConfigWatcher,
+};
 use crate::{
     command::dev::next::FileWatcher,
     composition::events::CompositionEvent,
@@ -33,16 +40,8 @@ use crate::{
     },
 };
 
-use super::{
-    binary::{RouterLog, RunRouterBinary, RunRouterBinaryError},
-    config::{remote::RemoteRouterConfig, ReadRouterConfigError, RouterAddress, RunRouterConfig},
-    hot_reload::{HotReloadEvent, HotReloadWatcher, RouterUpdateEvent},
-    install::{InstallRouter, InstallRouterError},
-    watchers::router_config::RouterConfigWatcher,
-};
-
 pub struct RunRouter<S> {
-    state: S,
+    pub(crate) state: S,
 }
 
 impl Default for RunRouter<state::Install> {
@@ -86,6 +85,12 @@ impl RunRouter<state::LoadLocalConfig> {
             .with_address(router_address)
             .with_config(read_file_impl, config_path.as_ref())
             .await?;
+        if let Some(config_path) = config_path.clone() {
+            infoln!(
+                "Watching {} for changes",
+                config_path.as_std_path().display()
+            );
+        }
         Ok(RunRouter {
             state: state::LoadRemoteConfig {
                 binary: self.state.binary,
@@ -158,6 +163,7 @@ impl RunRouter<state::Run> {
             .call(
                 WriteFileRequest::builder()
                     .path(hot_reload_config_path.clone())
+                    .contents(Vec::from(self.state.config.raw_config()))
                     .build(),
             )
             .await

src/composition/pipeline.rs

@@ -6,18 +6,6 @@ use rover_client::shared::GraphRef;
 use tempfile::tempdir;
 use tower::MakeService;
 
-use crate::{
-    options::{LicenseAccepter, ProfileOpt},
-    utils::{
-        client::StudioClientConfig,
-        effect::{
-            exec::ExecCommand, install::InstallBinary, introspect::IntrospectSubgraph,
-            read_file::ReadFile, read_stdin::ReadStdin, write_file::WriteFile,
-        },
-        parsers::FileDescriptorType,
-    },
-};
-
 use super::{
     runner::{CompositionRunner, Runner},
     supergraph::{
@@ -32,6 +20,18 @@ use super::{
     },
     CompositionError, CompositionSuccess,
 };
+use crate::composition::pipeline::CompositionPipelineError::FederationOneWithFederationTwoSubgraphs;
+use crate::{
+    options::{LicenseAccepter, ProfileOpt},
+    utils::{
+        client::StudioClientConfig,
+        effect::{
+            exec::ExecCommand, install::InstallBinary, introspect::IntrospectSubgraph,
+            read_file::ReadFile, read_stdin::ReadStdin, write_file::WriteFile,
+        },
+        parsers::FileDescriptorType,
+    },
+};
 
 #[derive(thiserror::Error, Debug)]
 pub enum CompositionPipelineError {
@@ -52,10 +52,12 @@ pub enum CompositionPipelineError {
     },
     #[error("Failed to install the supergraph binary.\n{}", .0)]
     InstallSupergraph(#[from] InstallSupergraphError),
+    #[error("Federation 1 version specified, but supergraph schema includes Federation 2 subgraphs: {0:?}")]
+    FederationOneWithFederationTwoSubgraphs(Vec<String>),
 }
 
 pub struct CompositionPipeline<State> {
-    state: State,
+    pub(crate) state: State,
 }
 
 impl Default for CompositionPipeline<state::Init> {
@@ -98,10 +100,12 @@ impl CompositionPipeline<state::Init> {
             }
             FileDescriptorType::Stdin => None,
         });
+        eprintln!("merging supergraph schema files");
         let resolver = SupergraphConfigResolver::default()
             .load_remote_subgraphs(fetch_remote_subgraphs_factory, graph_ref.as_ref())
             .await?
             .load_from_file_descriptor(read_stdin_impl, supergraph_yaml.as_ref())?;
+        eprintln!("supergraph config loaded successfully");
         Ok(CompositionPipeline {
             state: state::ResolveFederationVersion {
                 resolver,
@@ -134,11 +138,27 @@ impl CompositionPipeline<state::ResolveFederationVersion> {
                 &SubgraphPrompt::default(),
             )
             .await?;
-        let federation_version = federation_version.unwrap_or_else(|| {
+        let fed_two_subgraphs = fully_resolved_supergraph_config
+            .subgraphs()
+            .iter()
+            .filter_map(|(name, subgraph)| {
+                if subgraph.is_fed_two {
+                    Some(name.clone())
+                } else {
+                    None
+                }
+            })
+            .collect::<Vec<String>>();
+        let federation_version = if let Some(fed_version) = federation_version {
+            if !fed_two_subgraphs.is_empty() && fed_version.is_fed_one() {
+                return Err(FederationOneWithFederationTwoSubgraphs(fed_two_subgraphs));
+            }
+            fed_version
+        } else {
             fully_resolved_supergraph_config
                 .federation_version()
                 .clone()
-        });
+        };
         Ok(CompositionPipeline {
             state: state::InstallSupergraph {
                 resolver: self.state.resolver,

src/composition/supergraph/config/federation.rs

@@ -6,9 +6,8 @@ use std::marker::PhantomData;
 use apollo_federation_types::config::{FederationVersion, SupergraphConfig};
 use derive_getters::Getters;
 
-use crate::command::supergraph::compose::do_compose::SupergraphComposeOpts;
-
 use super::full::FullyResolvedSubgraph;
+use crate::command::supergraph::compose::do_compose::SupergraphComposeOpts;
 
 mod state {
     #[derive(Clone, Debug)]
@@ -166,9 +165,8 @@ mod tests {
     use apollo_federation_types::config::{FederationVersion, SubgraphConfig, SupergraphConfig};
     use speculoos::prelude::*;
 
-    use crate::composition::supergraph::config::{full::FullyResolvedSubgraph, scenario::*};
-
     use super::FederationVersionResolverFromSupergraphConfig;
+    use crate::composition::supergraph::config::{full::FullyResolvedSubgraph, scenario::*};
 
     /// Test showing that federation version is selected from the user-specified fed version
     /// over local supergraph config or resolved subgraphs
@@ -189,7 +187,7 @@ mod tests {
         let supergraph_config =
             SupergraphConfig::new(unresolved_subgraphs, Some(FederationVersion::LatestFedOne));
 
-        let resolved_subgraphs = vec![(
+        let resolved_subgraphs = [(
             subgraph_name.to_string(),
             FullyResolvedSubgraph::builder()
                 .schema(subgraph_scenario.sdl)
@@ -222,7 +220,7 @@ mod tests {
         let supergraph_config =
             SupergraphConfig::new(unresolved_subgraphs, Some(FederationVersion::LatestFedTwo));
 
-        let resolved_subgraphs = vec![(
+        let resolved_subgraphs = [(
             subgraph_name.to_string(),
             FullyResolvedSubgraph::builder()
                 .schema(subgraph_scenario.sdl)
@@ -254,7 +252,7 @@ mod tests {
         let federation_version_resolver = FederationVersionResolverFromSupergraphConfig::default();
         let supergraph_config = SupergraphConfig::new(unresolved_subgraphs, None);
 
-        let resolved_subgraphs = vec![(
+        let resolved_subgraphs = [(
             subgraph_name.to_string(),
             FullyResolvedSubgraph::builder()
                 .schema(subgraph_scenario.sdl)