Merge pull request #132 from itowlson/manifest-v2

Manifest v2 support
fermyon · Oct 26, 2023 · a0f3ee0 · a0f3ee0
2 parents 16e4113 + 02c1b02
commit a0f3ee0
Show file tree

Hide file tree

Showing 8 changed files with 685 additions and 773 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -32,15 +32,15 @@ semver = "1.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.82"
 sha2 = "0.10.2"
-spin-app = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-common = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-loader = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-manifest = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-http = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-oci = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-trigger = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-spin-trigger-http = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
-terminal = { git = "https://github.com/fermyon/spin", rev = "54c4d83987f0519ef885b532d577845443d4d61b" }
+spin-app = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-common = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-loader = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-manifest = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-http = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-oci = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-trigger = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+spin-trigger-http = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
+terminal = { git = "https://github.com/fermyon/spin", rev = "57b004cf7fa2b6ad545bd8292f2745ed6424f05b" }
 tempfile = "3.3.0"
 url = "2.3"
 uuid = { version = "1.3", features = ["v4"] }

diff --git a/src/commands/deploy.rs b/src/commands/deploy.rs
@@ -12,7 +12,6 @@ use cloud_openapi::models::{
 use oci_distribution::{token_cache, Reference, RegistryOperation};
 use spin_common::arg_parser::parse_kv;
 use spin_http::{app_info::AppInfo, routes::RoutePattern};
-use spin_manifest::ApplicationTrigger;
 use tokio::fs;
 use tracing::instrument;
 
@@ -303,53 +302,47 @@ impl DeployCommand {
     async fn load_cloud_app(&self, working_dir: &Path) -> Result<DeployableApp, anyhow::Error> {
         let app_source = self.resolve_app_source();
 
-        match &app_source {
+        let locked_app = match &app_source {
             AppSource::File(app_file) => {
-                let cfg_any = spin_loader::local::raw_manifest_from_file(&app_file).await?;
-                let cfg = cfg_any.into_v1();
-
-                match cfg.info.trigger {
-                    ApplicationTrigger::Http(_) => {}
-                    ApplicationTrigger::Redis(_) => bail!("Redis triggers are not supported"),
-                    ApplicationTrigger::External(_) => bail!("External triggers are not supported"),
-                }
-
-                let app = spin_loader::from_file(app_file, Some(working_dir)).await?;
-                let locked_app = spin_trigger::locked::build_locked_app(app, working_dir)?;
-
-                Ok(DeployableApp(locked_app))
+                spin_loader::from_file(
+                    &app_file,
+                    spin_loader::FilesMountStrategy::Copy(working_dir.to_owned()),
+                )
+                .await?
             }
             AppSource::OciRegistry(reference) => {
                 let mut oci_client = spin_oci::Client::new(false, None)
                     .await
                     .context("cannot create registry client")?;
 
-                let locked_app = spin_oci::OciLoader::new(working_dir)
+                spin_oci::OciLoader::new(working_dir)
                     .load_app(&mut oci_client, reference)
-                    .await?;
-
-                let unsupported_triggers = locked_app
-                    .triggers
-                    .iter()
-                    .filter(|t| t.trigger_type != "http")
-                    .map(|t| format!("'{}'", t.trigger_type))
-                    .collect::<Vec<_>>();
-                if !unsupported_triggers.is_empty() {
-                    bail!(
-                        "Non-HTTP triggers are not supported - app uses {}",
-                        unsupported_triggers.join(", ")
-                    );
-                }
-
-                Ok(DeployableApp(locked_app))
+                    .await?
             }
             AppSource::None => {
                 anyhow::bail!("Default file '{DEFAULT_MANIFEST_FILE}' not found.");
             }
             AppSource::Unresolvable(err) => {
                 anyhow::bail!("{err}");
             }
+        };
+
+        let unsupported_triggers = locked_app
+            .triggers
+            .iter()
+            .filter(|t| t.trigger_type != "http")
+            .map(|t| format!("'{}'", t.trigger_type))
+            .collect::<Vec<_>>();
+        if !unsupported_triggers.is_empty() {
+            bail!(
+                "Non-HTTP triggers are not supported - app uses {}",
+                unsupported_triggers.join(", ")
+            );
         }
+
+        let locked_app = ensure_http_base_set(locked_app);
+
+        Ok(DeployableApp(locked_app))
     }
 
     async fn validate_deployment_environment(
@@ -464,6 +457,25 @@ impl DeployCommand {
     }
 }
 
+// Spin now allows HTTP apps to omit the base path, but Cloud
+// doesn't yet like this. This works around that by defaulting
+// base if not set. (We don't check trigger type because by the
+// time this is called we know it's HTTP.)
+fn ensure_http_base_set(
+    mut locked_app: spin_app::locked::LockedApp,
+) -> spin_app::locked::LockedApp {
+    if let Some(trigger) = locked_app
+        .metadata
+        .entry("trigger")
+        .or_insert_with(|| serde_json::Value::Object(Default::default()))
+        .as_object_mut()
+    {
+        trigger.entry("base").or_insert_with(|| "/".into());
+    }
+
+    locked_app
+}
+
 #[derive(Debug, PartialEq, Eq)]
 enum AppSource {
     None,
@@ -989,9 +1001,16 @@ fn print_available_routes(app_base_url: &Url, base: &str, routes: &[HttpRoute])
     let app_base_url = app_base_url.to_string();
     let route_prefix = app_base_url.strip_suffix('/').unwrap_or(&app_base_url);
 
+    // Ensure base starts with a /
+    let base = if !base.starts_with('/') {
+        format!("/{base}")
+    } else {
+        base.to_owned()
+    };
+
     println!("Available Routes:");
     for component in routes {
-        let route = RoutePattern::from(base, &component.route_pattern);
+        let route = RoutePattern::from(&base, &component.route_pattern);
         println!("  {}: {}{}", component.id, route_prefix, route);
         if let Some(description) = &component.description {
             println!("    {}", description);
@@ -1178,4 +1197,65 @@ mod test {
             sanitize_app_version("e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855e3b")
         );
     }
+
+    fn deploy_cmd_for_test_file(filename: &str) -> DeployCommand {
+        let path = PathBuf::from(env!("CARGO_MANIFEST_DIR"))
+            .join("testdata")
+            .join(filename);
+        DeployCommand {
+            app_source: None,
+            file_source: Some(path),
+            registry_source: None,
+            build: false,
+            readiness_timeout_secs: 60,
+            deployment_env_id: None,
+            key_values: vec![],
+            variables: vec![],
+        }
+    }
+
+    fn get_trigger_base(mut app: DeployableApp) -> String {
+        let serde_json::map::Entry::Occupied(trigger) = app.0.metadata.entry("trigger") else {
+            panic!("Expected trigger metadata but entry was vacant");
+        };
+        let base = trigger
+            .get()
+            .as_object()
+            .unwrap()
+            .get("base")
+            .expect("Manifest should have had a base but didn't");
+        base.as_str()
+            .expect("HTTP base should have been a string but wasn't")
+            .to_owned()
+    }
+
+    #[tokio::test]
+    async fn if_http_base_is_set_then_it_is_respected() {
+        let temp_dir = tempfile::tempdir().unwrap();
+
+        let cmd = deploy_cmd_for_test_file("based_v1.toml");
+        let app = cmd.load_cloud_app(temp_dir.path()).await.unwrap();
+        let base = get_trigger_base(app);
+        assert_eq!("/base", base);
+
+        let cmd = deploy_cmd_for_test_file("based_v2.toml");
+        let app = cmd.load_cloud_app(temp_dir.path()).await.unwrap();
+        let base = get_trigger_base(app);
+        assert_eq!("/base", base);
+    }
+
+    #[tokio::test]
+    async fn if_http_base_is_not_set_then_it_is_inserted() {
+        let temp_dir = tempfile::tempdir().unwrap();
+
+        let cmd = deploy_cmd_for_test_file("unbased_v1.toml");
+        let app = cmd.load_cloud_app(temp_dir.path()).await.unwrap();
+        let base = get_trigger_base(app);
+        assert_eq!("/", base);
+
+        let cmd = deploy_cmd_for_test_file("unbased_v2.toml");
+        let app = cmd.load_cloud_app(temp_dir.path()).await.unwrap();
+        let base = get_trigger_base(app);
+        assert_eq!("/", base);
+    }
 }
diff --git a/testdata/based_v1.toml b/testdata/based_v1.toml
@@ -0,0 +1,10 @@
+spin_manifest_version = "1"
+name = "based-v1"
+version = "0.0.1"
+trigger = { type = "http", base = "/base" }
+
+[[component]]
+id = "dummy"
+source = "dummy.not-actually-wasm"
+[component.trigger]
+route = "/dummy"
diff --git a/testdata/based_v2.toml b/testdata/based_v2.toml
@@ -0,0 +1,12 @@
+spin_manifest_version = 2
+
+[application]
+name = "unbased_v1"
+version = "0.1.0"
+
+[application.trigger.http]
+base = "/base"
+
+[[trigger.http]]
+route = "/..."
+component = { source = "dummy.not-actually-wasm" }
diff --git a/testdata/dummy.not-actually-wasm b/testdata/dummy.not-actually-wasm
@@ -0,0 +1 @@
+Dummy file that only exists because the loader checks for the existence of component sources
diff --git a/testdata/unbased_v1.toml b/testdata/unbased_v1.toml
@@ -0,0 +1,10 @@
+spin_manifest_version = "1"
+name = "based-v1"
+version = "0.0.1"
+trigger = { type = "http" }
+
+[[component]]
+id = "dummy"
+source = "dummy.not-actually-wasm"
+[component.trigger]
+route = "/dummy"
diff --git a/testdata/unbased_v2.toml b/testdata/unbased_v2.toml
@@ -0,0 +1,9 @@
+spin_manifest_version = 2
+
+[application]
+name = "unbased_v1"
+version = "0.1.0"
+
+[[trigger.http]]
+route = "/..."
+component = { source = "dummy.not-actually-wasm" }