Repository: fermyon/spin (rust)

Issue:
Rename `spin_version` to `spin_manifest_version` in Spin application manifest
## Issue
Spin application manifests (`spin.toml`) must specify what application manifest version it is using. This is set by setting the [`spin_version` in a `spin.toml`](https://github.com/fermyon/spin/blob/7cad829d6c87d6956a049bb6fa7e8d3711b6d70c/examples/http-rust/spin.toml#L1). This field suggests that it is declaring the version of Spin that it runs on when it is really declaring the version of the manifest TOML that it is. We should rename this field to `spin_manifest_version` to make this clear.

## Proposed solution
1) non-breaking: Ideally, we could add an alias to the `spin_version` field of the manifest so that the name change would not be breaking. This may not be possible with `serde` defaults. Instead, we may need to write a custom deserializer that deserializes that field appropriately and calls to the default deserializer for the remaining fields. 

2) Alternatively, we could make it a breaking change with a major release.

Pull request description:
Rename `spin_version` to `spin_manifest_version` in Spin.toml
Ensures backwards compatibility.

Required bringing in `spin-app` crate to `build` and `loader` crates to access `LockedVersion` type.

fixes #1069 
Thank you @lann for suggested implementation and @fibonacci1729 for pairing!

Patch:
diff --git a/crates/bindle/src/expander.rs b/crates/bindle/src/expander.rs
index ef6d8999..5001384b 100644
--- a/crates/bindle/src/expander.rs
+++ b/crates/bindle/src/expander.rs
@@ -20,7 +20,7 @@ pub async fn expand_manifest(
     let app_file = absolutize(app_file)?;
     let manifest = spin_loader::local::raw_manifest_from_file(&app_file).await?;
     validate_raw_app_manifest(&manifest)?;
-    let local_schema::RawAppManifestAnyVersion::V1(manifest) = manifest;
+    let manifest = manifest.into_v1();
     let app_dir = parent_dir(&app_file)?;
 
     // * create a new spin.toml-like document where
diff --git a/crates/build/src/lib.rs b/crates/build/src/lib.rs
index 16ffb1c5..6f97793b 100644
--- a/crates/build/src/lib.rs
+++ b/crates/build/src/lib.rs
@@ -16,7 +16,7 @@ pub async fn build(manifest_file: &Path) -> Result<()> {
     let manifest_text = tokio::fs::read_to_string(manifest_file)
         .await
         .with_context(|| format!("Cannot read manifest file from {}", manifest_file.display()))?;
-    let BuildAppInfoAnyVersion::V1(app) = toml::from_str(&manifest_text)?;
+    let app = toml::from_str(&manifest_text).map(BuildAppInfoAnyVersion::into_v1)?;
     let app_dir = parent_dir(manifest_file)?;
 
     if app.components.iter().all(|c| c.build.is_none()) {
diff --git a/crates/build/src/manifest.rs b/crates/build/src/manifest.rs
index 2eea6dc5..ae04ea1a 100644
--- a/crates/build/src/manifest.rs
+++ b/crates/build/src/manifest.rs
@@ -1,12 +1,30 @@
-use std::path::PathBuf;
-
 use serde::{Deserialize, Serialize};
+use spin_loader::local::config::FixedStringVersion;
+use std::path::PathBuf;
 
-#[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(tag = "spin_version")]
+#[derive(Clone, Debug, Deserialize)]
+#[serde(untagged)]
 pub(crate) enum BuildAppInfoAnyVersion {
-    #[serde(rename = "1")]
-    V1(BuildAppInfoV1),
+    V1Old {
+        #[allow(dead_code)]
+        spin_version: FixedStringVersion<1>,
+        #[serde(flatten)]
+        manifest: BuildAppInfoV1,
+    },
+    V1New {
+        #[allow(dead_code)]
+        spin_manifest_version: FixedStringVersion<1>,
+        #[serde(flatten)]
+        manifest: BuildAppInfoV1,
+    },
+}
+impl BuildAppInfoAnyVersion {
+    pub fn into_v1(self) -> BuildAppInfoV1 {
+        match self {
+            BuildAppInfoAnyVersion::V1New { manifest, .. } => manifest,
+            BuildAppInfoAnyVersion::V1Old { manifest, .. } => manifest,
+        }
+    }
 }
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
diff --git a/crates/loader/src/local/config.rs b/crates/loader/src/local/config.rs
index b7f0e0ac..fa1fc72c 100644
--- a/crates/loader/src/local/config.rs
+++ b/crates/loader/src/local/config.rs
@@ -22,12 +22,43 @@ pub(crate) type RawAppManifestAnyVersionPartial = RawAppManifestAnyVersionImpl<t
 pub(crate) type RawComponentManifestPartial = RawComponentManifestImpl<toml::Value>;
 
 /// Container for any version of the manifest.
-#[derive(Clone, Debug, Deserialize, Serialize)]
-#[serde(tag = "spin_version")]
+#[derive(Clone, Debug, Deserialize)]
+#[serde(untagged)]
 pub enum RawAppManifestAnyVersionImpl<C> {
-    /// A manifest with API version 1.
-    #[serde(rename = "1")]
-    V1(RawAppManifestImpl<C>),
+    /// Old Spin manifest versioned with `spin_version` key
+    V1Old {
+        /// Version key name
+        spin_version: FixedStringVersion<1>,
+        /// Manifest
+        #[serde(flatten)]
+        manifest: RawAppManifestImpl<C>,
+    },
+    /// New Spin manifest versioned with `spin_manifest_version` key
+    V1New {
+        /// Version key name
+        spin_manifest_version: FixedStringVersion<1>,
+        /// Manifest
+        #[serde(flatten)]
+        manifest: RawAppManifestImpl<C>,
+    },
+}
+
+impl<C> RawAppManifestAnyVersionImpl<C> {
+    /// Converts `RawAppManifestAnyVersionImpl` into underlying V1 manifest
+    pub fn into_v1(self) -> RawAppManifestImpl<C> {
+        match self {
+            RawAppManifestAnyVersionImpl::V1New { manifest, .. } => manifest,
+            RawAppManifestAnyVersionImpl::V1Old { manifest, .. } => manifest,
+        }
+    }
+
+    /// Returns a reference to the underlying V1 manifest
+    pub fn as_v1(&self) -> &RawAppManifestImpl<C> {
+        match self {
+            RawAppManifestAnyVersionImpl::V1New { manifest, .. } => manifest,
+            RawAppManifestAnyVersionImpl::V1Old { manifest, .. } => manifest,
+        }
+    }
 }
 
 /// Application configuration local file format.
@@ -176,3 +207,27 @@ pub struct FileComponentUrlSource {
     /// SHA256 digest, in the form `sha256:...`
     pub digest: String,
 }
+
+/// FixedStringVersion represents a schema version field with a const value.
+#[derive(Clone, Debug, Default, Serialize, Deserialize)]
+#[serde(into = "String", try_from = "String")]
+pub struct FixedStringVersion<const V: usize>;
+
+impl<const V: usize> From<FixedStringVersion<V>> for String {
+    fn from(_: FixedStringVersion<V>) -> String {
+        V.to_string()
+    }
+}
+
+impl<const V: usize> TryFrom<String> for FixedStringVersion<V> {
+    type Error = String;
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        let value: usize = value
+            .parse()
+            .map_err(|err| format!("invalid version: {}", err))?;
+        if value != V {
+            return Err(format!("invalid version {} != {}", value, V));
+        }
+        Ok(Self)
+    }
+}
diff --git a/crates/loader/src/local/mod.rs b/crates/loader/src/local/mod.rs
index a7b132fd..016a74e5 100644
--- a/crates/loader/src/local/mod.rs
+++ b/crates/loader/src/local/mod.rs
@@ -101,9 +101,8 @@ async fn prepare_any_version(
     base_dst: Option<impl AsRef<Path>>,
     bindle_connection: &Option<BindleConnectionInfo>,
 ) -> Result<Application> {
-    match raw {
-        RawAppManifestAnyVersion::V1(raw) => prepare(raw, src, base_dst, bindle_connection).await,
-    }
+    let manifest = raw.into_v1();
+    prepare(manifest, src, base_dst, bindle_connection).await
 }
 
 /// Iterates over a vector of RawComponentManifest structs and throws an error if any component ids are duplicated
@@ -122,13 +121,11 @@ fn error_on_duplicate_ids(components: Vec<RawComponentManifest>) -> Result<()> {
 
 /// Validate fields in raw app manifest
 pub fn validate_raw_app_manifest(raw: &RawAppManifestAnyVersion) -> Result<()> {
-    match raw {
-        RawAppManifestAnyVersion::V1(raw) => {
-            raw.components
-                .iter()
-                .try_for_each(|c| validate_allowed_http_hosts(&c.wasm.allowed_http_hosts))?;
-        }
-    }
+    let manifest = raw.as_v1();
+    manifest
+        .components
+        .iter()
+        .try_for_each(|c| validate_allowed_http_hosts(&c.wasm.allowed_http_hosts))?;
     Ok(())
 }
 
@@ -405,7 +402,7 @@ impl ComponentDigest {
 fn resolve_partials(
     partially_parsed: RawAppManifestAnyVersionPartial,
 ) -> Result<RawAppManifestAnyVersion> {
-    let RawAppManifestAnyVersionPartial::V1(manifest) = partially_parsed;
+    let manifest = partially_parsed.into_v1();
 
     let app_trigger = &manifest.info.trigger;
     let components = manifest
@@ -414,11 +411,15 @@ fn resolve_partials(
         .map(|c| resolve_partial_component(app_trigger, c))
         .collect::<Result<_>>()?;
 
-    Ok(RawAppManifestAnyVersion::V1(RawAppManifest {
-        info: manifest.info,
-        components,
-        variables: manifest.variables,
-    }))
+    // Only concerned with preserving manifest.
+    Ok(RawAppManifestAnyVersion::V1New {
+        manifest: RawAppManifest {
+            info: manifest.info,
+            components,
+            variables: manifest.variables,
+        },
+        spin_manifest_version: config::FixedStringVersion::default(),
+    })
 }
 
 fn resolve_partial_component(
@@ -494,8 +495,7 @@ source = "nonexistent.wasm"
     #[test]
     fn can_parse_http_trigger() {
         let m = load_test_manifest(r#"{ type = "http", base = "/" }"#, r#"route = "/...""#);
-
-        let RawAppManifestAnyVersion::V1(m1) = m;
+        let m1 = m.into_v1();
         let t = &m1.info.trigger;
         let ct = &m1.components[0].trigger;
         assert!(matches!(t, ApplicationTrigger::Http(_)));
@@ -509,7 +509,7 @@ source = "nonexistent.wasm"
             r#"channel = "chan""#,
         );
 
-        let RawAppManifestAnyVersion::V1(m1) = m;
+        let m1 = m.into_v1();
         let t = m1.info.trigger;
         let ct = &m1.components[0].trigger;
         assert!(matches!(t, ApplicationTrigger::Redis(_)));
@@ -520,7 +520,7 @@ source = "nonexistent.wasm"
     fn can_parse_unknown_trigger() {
         let m = load_test_manifest(r#"{ type = "pounce" }"#, r#"on = "MY KNEES""#);
 
-        let RawAppManifestAnyVersion::V1(m1) = m;
+        let m1 = m.into_v1();
         let t = m1.info.trigger;
         let ct = &m1.components[0].trigger;
         assert!(matches!(t, ApplicationTrigger::External(_)));
@@ -534,7 +534,7 @@ source = "nonexistent.wasm"
             r#"route = "over the cat tree and out of the sun""#,
         );
 
-        let RawAppManifestAnyVersion::V1(m1) = m;
+        let m1 = m.into_v1();
         let t = m1.info.trigger;
         let ct = &m1.components[0].trigger;
         assert!(matches!(t, ApplicationTrigger::External(_)));
diff --git a/crates/trigger/src/cli.rs b/crates/trigger/src/cli.rs
index 3583611b..f139310b 100644
--- a/crates/trigger/src/cli.rs
+++ b/crates/trigger/src/cli.rs
@@ -58,7 +58,7 @@ where
     )]
     pub cache: Option<PathBuf>,
 
-    /// Print output to stdout/stderr only for given component(s)
+    /// Print output for given component(s) to stdout/stderr
     #[clap(
         name = FOLLOW_LOG_OPT,
         long = "follow",
@@ -66,6 +66,11 @@ where
     )]
     pub follow_components: Vec<String>,
 
+    /// Print all component output to stdout/stderr
+    /// Deprecated
+    #[clap(long = "follow-all")]
+    pub follow_all_components: bool,
+
     /// Silence all component output to stdout/stderr
     #[clap(
         long = "quiet",
@@ -172,6 +177,9 @@ where
     }
 
     pub fn follow_components(&self) -> FollowComponents {
+        if self.follow_all_components {
+            eprintln!("NOTE: --follow-all has been deprecated. All component output is now printed to stdout by default.")
+        }
         if self.silence_component_logs {
             FollowComponents::None
         } else if self.follow_components.is_empty() {
diff --git a/examples/http-rust/Cargo.lock b/examples/http-rust/Cargo.lock
index d3593d5b..d5c52883 100644
--- a/examples/http-rust/Cargo.lock
+++ b/examples/http-rust/Cargo.lock
@@ -154,7 +154,7 @@ dependencies = [
 
 [[package]]
 name = "spin-sdk"
-version = "0.7.1"
+version = "0.8.0"
 dependencies = [
  "anyhow",
  "bytes",
diff --git a/src/bin/spin.rs b/src/bin/spin.rs
index a3164adf..419602d4 100644
--- a/src/bin/spin.rs
+++ b/src/bin/spin.rs
@@ -55,9 +55,11 @@ enum SpinApp {
     Bindle(BindleCommands),
     #[clap(subcommand)]
     Cloud(CloudCommands),
-    // acts as a cross-level subcommand shortcut -> `spin cloud deploy`
+    // hidden to "work" as a cross-level subcommand alias -> `spin cloud deploy`
+    #[clap(hide = true)]
     Deploy(DeployCommand),
-    // acts as a cross-level subcommand shortcut -> `spin cloud login`
+    // hidden to "work" as a cross-level subcommand alias -> `spin cloud login`
+    #[clap(hide = true)]
     Login(LoginCommand),
     #[clap(subcommand, alias = "oci")]
     Registry(RegistryCommands),
diff --git a/src/commands/deploy.rs b/src/commands/deploy.rs
index 47d4b0c3..927a2c38 100644
--- a/src/commands/deploy.rs
+++ b/src/commands/deploy.rs
@@ -15,7 +15,7 @@ use spin_http::routes::RoutePattern;
 use spin_http::AppInfo;
 use spin_http::WELL_KNOWN_PREFIX;
 use spin_loader::bindle::BindleConnectionInfo;
-use spin_loader::local::config::{RawAppManifest, RawAppManifestAnyVersion};
+use spin_loader::local::config::RawAppManifest;
 use spin_loader::local::{assets, config, parent_dir};
 use spin_manifest::ApplicationTrigger;
 use spin_manifest::{HttpTriggerConfiguration, TriggerConfig};
@@ -188,7 +188,7 @@ impl DeployCommand {
 
     async fn deploy_hippo(self, login_connection: LoginConnection) -> Result<()> {
         let cfg_any = spin_loader::local::raw_manifest_from_file(&self.app).await?;
-        let RawAppManifestAnyVersion::V1(cfg) = cfg_any;
+        let cfg = cfg_any.into_v1();
 
         ensure!(!cfg.components.is_empty(), "No components in spin.toml!");
 
@@ -318,7 +318,7 @@ impl DeployCommand {
         let client = CloudClient::new(connection_config.clone());
 
         let cfg_any = spin_loader::local::raw_manifest_from_file(&self.app).await?;
-        let RawAppManifestAnyVersion::V1(cfg) = cfg_any;
+        let cfg = cfg_any.into_v1();
 
         ensure!(!cfg.components.is_empty(), "No components in spin.toml!");
 


Test patch:
diff --git a/crates/loader/src/local/tests.rs b/crates/loader/src/local/tests.rs
index 06a2e070..e9b17cfb 100644
--- a/crates/loader/src/local/tests.rs
+++ b/crates/loader/src/local/tests.rs
@@ -47,12 +47,34 @@ async fn test_from_local_source() -> Result<()> {
     Ok(())
 }
 
+#[test]
+fn test_manifest_v1_signatures() -> Result<()> {
+    const MANIFEST: &str = include_str!("../../tests/valid-manifest.toml");
+    let ageless = MANIFEST
+        .replace("spin_version", "temp")
+        .replace("spin_manifest_version", "temp");
+    let v1_old = ageless.replace("temp", "spin_version");
+    let v1_new = ageless.replace("temp", "spin_manifest_version");
+    into_v1_manifest(&v1_old, "chain-of-command")?;
+    into_v1_manifest(&v1_new, "chain-of-command")?;
+    Ok(())
+}
+
+fn into_v1_manifest(spin_versioned_manifest: &str, app_name: &str) -> Result<()> {
+    use config::RawAppManifestAnyVersionImpl;
+    let raw: RawAppManifestAnyVersionImpl<toml::Value> =
+        toml::from_slice(spin_versioned_manifest.as_bytes())?;
+    let manifest = raw.into_v1();
+    assert_eq!(manifest.info.name, app_name);
+    Ok(())
+}
+
 #[test]
 fn test_manifest() -> Result<()> {
     const MANIFEST: &str = include_str!("../../tests/valid-manifest.toml");
 
     let cfg_any: RawAppManifestAnyVersion = raw_manifest_from_str(MANIFEST)?;
-    let RawAppManifestAnyVersion::V1(cfg) = cfg_any;
+    let cfg = cfg_any.into_v1();
 
     assert_eq!(cfg.info.name, "chain-of-command");
     assert_eq!(cfg.info.version, "6.11.2");
@@ -184,8 +206,8 @@ fn test_unknown_version_is_rejected() {
 
     let e = cfg.unwrap_err().to_string();
     assert!(
-        e.contains("spin_version"),
-        "Expected error to mention `spin_version`"
+        e.contains("RawAppManifestAnyVersionImpl"),
+        "Expected error to mention `RawAppManifestAnyVersionImpl`"
     );
 }
 
@@ -197,7 +219,7 @@ fn test_wagi_executor_with_custom_entrypoint() -> Result<()> {
     const EXPECTED_DEFAULT_ARGV: &str = "${SCRIPT_NAME} ${ARGS}";
 
     let cfg_any: RawAppManifestAnyVersion = raw_manifest_from_str(MANIFEST)?;
-    let RawAppManifestAnyVersion::V1(cfg) = cfg_any;
+    let cfg = cfg_any.into_v1();
 
     let http_config: HttpConfig = cfg.components[0].trigger.clone().try_into()?;
 
diff --git a/tests/integration.rs b/tests/integration.rs
index 963ee4e7..a41cafc8 100644
--- a/tests/integration.rs
+++ b/tests/integration.rs
@@ -2,10 +2,7 @@
 mod integration_tests {
     use anyhow::{Context, Result};
     use hyper::{Body, Client, Response};
-    use spin_loader::local::{
-        config::{RawAppManifestAnyVersion, RawModuleSource},
-        raw_manifest_from_file,
-    };
+    use spin_loader::local::{config::RawModuleSource, raw_manifest_from_file};
     use std::{
         collections::HashMap,
         ffi::OsStr,
@@ -994,7 +991,7 @@ mod integration_tests {
     /// in `spin.toml` inside `dir`.
     async fn do_test_build_command(dir: impl AsRef<Path>) -> Result<()> {
         let manifest_file = dir.as_ref().join("spin.toml");
-        let RawAppManifestAnyVersion::V1(manifest) = raw_manifest_from_file(&manifest_file).await?;
+        let manifest = raw_manifest_from_file(&manifest_file).await?.into_v1();
 
         let mut sources = vec![];
         for component_manifest in manifest.components.iter() {