👻 Add options to customize behavior of unknown Wasm imports (#313)

Adds a new shared option to the CLI with three behavior modes:

```
      --unknown-import-behavior <UNKNOWN_IMPORT_BEHAVIOR>
          Set the behavior for unknown imports.

          Note that if a program only works with a non-default setting for this flag, it is unlikely to be publishable to Fastly.

          [default: link-error]

          Possible values:
          - link-error:   Unknown imports are rejected at link time (default behavior)
          - trap:         Unknown imports trap when called
          - zero-or-null: Unknown imports return zero or a null pointer, depending on the type
```

This ended up touching more files than I'd expect, because I had to make the integration test
harness bubble out link-time errors rather than `.expect()`ing them, so there are quite a few new
`?`s across the integration test suite.

Cargo.toml

@@ -23,6 +23,7 @@ opt-level = 1
 [workspace.dependencies]
 anyhow = "1.0.31"
 base64 = "0.21.2"
+clap = { version = "^4.0.18", features = ["derive"] }
 hyper = { version = "=0.14.26", features = ["full"] }
 itertools = "0.10.5"
 rustls = { version = "0.21.5", features = ["dangerous_configuration"] }

cli/Cargo.toml

@@ -35,7 +35,7 @@ base64 = { workspace = true }
 hyper = { workspace = true }
 itertools = { workspace = true }
 serde_json = { workspace = true }
-clap = { version = "^4.0.18", features = ["derive"] }
+clap = { workspace = true }
 rustls = { workspace = true }
 rustls-pemfile = { workspace = true }
 tls-listener = { version = "^0.7.0", features = ["rustls", "hyper-h1", "tokio-net", "rt"] }

cli/src/main.rs

@@ -247,6 +247,7 @@ async fn create_execution_context(
         args.profiling_strategy(),
         args.wasi_modules(),
         guest_profile_path,
+        args.unknown_import_behavior(),
     )?
     .with_log_stderr(args.log_stderr())
     .with_log_stdout(args.log_stdout());

cli/src/opts.rs

@@ -1,5 +1,7 @@
 //! Command line arguments.
 
+use viceroy_lib::config::UnknownImportBehavior;
+
 use {
     clap::{Args, Parser, Subcommand, ValueEnum},
     std::net::{IpAddr, Ipv4Addr},
@@ -90,6 +92,12 @@ pub struct SharedArgs {
     /// Set of experimental WASI modules to link against.
     #[arg(value_enum, long = "experimental_modules", required = false)]
     experimental_modules: Vec<ExperimentalModuleArg>,
+    /// Set the behavior for unknown imports.
+    ///
+    /// Note that if a program only works with a non-default setting for this flag, it is unlikely
+    /// to be publishable to Fastly.
+    #[arg(long = "unknown-import-behavior", value_enum, default_value_t = UnknownImportBehavior::LinkError)]
+    unknown_import_behavior: UnknownImportBehavior,
     /// Verbosity of logs for Viceroy. `-v` sets the log level to INFO,
     /// `-vv` to DEBUG, and `-vvv` to TRACE. This option will not take
     /// effect if you set RUST_LOG to a value before starting Viceroy
@@ -151,16 +159,21 @@ impl SharedArgs {
         self.log_stderr
     }
 
-    // Whether to enable wasmtime's builtin profiler.
+    /// Whether to enable wasmtime's builtin profiler.
     pub fn profiling_strategy(&self) -> ProfilingStrategy {
         self.profiler.unwrap_or(ProfilingStrategy::None)
     }
 
-    // Set of experimental wasi modules to link against.
+    /// Set of experimental wasi modules to link against.
     pub fn wasi_modules(&self) -> HashSet<ExperimentalModule> {
         self.experimental_modules.iter().map(|x| x.into()).collect()
     }
 
+    /// Unknown import behavior
+    pub fn unknown_import_behavior(&self) -> UnknownImportBehavior {
+        self.unknown_import_behavior
+    }
+
     /// Verbosity of logs for Viceroy. `-v` sets the log level to DEBUG and
     /// `-vv` to TRACE. This option will not take effect if you set RUST_LOG
     /// to a value before starting Viceroy

cli/tests/integration/async_io.rs

@@ -143,7 +143,7 @@ async fn async_io_methods() -> TestResult {
         .await;
 
     // request_count is 0 here
-    let resp = test.against_empty().await;
+    let resp = test.against_empty().await?;
 
     assert_eq!(resp.status(), StatusCode::OK);
     assert_eq!(resp.headers()["Simple-Ready"], "false");
@@ -154,7 +154,7 @@ async fn async_io_methods() -> TestResult {
     barrier.wait().await;
 
     request_count.store(1, Ordering::Relaxed);
-    let resp = test.against_empty().await;
+    let resp = test.against_empty().await?;
     assert_eq!(resp.status(), StatusCode::OK);
     assert_eq!(resp.headers()["Simple-Ready"], "true");
     assert_eq!(resp.headers()["Read-Ready"], "false");
@@ -165,7 +165,7 @@ async fn async_io_methods() -> TestResult {
     barrier.wait().await;
 
     request_count.store(2, Ordering::Relaxed);
-    let resp = test.against_empty().await;
+    let resp = test.against_empty().await?;
     assert_eq!(resp.status(), StatusCode::OK);
     assert_eq!(resp.headers()["Simple-Ready"], "false");
     assert_eq!(resp.headers()["Read-Ready"], "true");
@@ -174,7 +174,7 @@ async fn async_io_methods() -> TestResult {
     barrier.wait().await;
 
     request_count.store(3, Ordering::Relaxed);
-    let resp = test.against_empty().await;
+    let resp = test.against_empty().await?;
     assert_eq!(resp.status(), StatusCode::OK);
     assert_eq!(resp.headers()["Simple-Ready"], "false");
     assert_eq!(resp.headers()["Read-Ready"], "false");
@@ -191,7 +191,7 @@ async fn async_io_methods() -> TestResult {
                 .body(Body::empty())
                 .unwrap(),
         )
-        .await;
+        .await?;
     assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR);
 
     let resp = test
@@ -201,7 +201,7 @@ async fn async_io_methods() -> TestResult {
                 .body(Body::empty())
                 .unwrap(),
         )
-        .await;
+        .await?;
     assert_eq!(resp.status(), StatusCode::OK);
     assert_eq!(resp.headers()["Ready-Index"], "timeout");
 

cli/tests/integration/body.rs

@@ -7,7 +7,9 @@ use {
 
 #[tokio::test(flavor = "multi_thread")]
 async fn bodies_can_be_written_and_appended() -> TestResult {
-    let resp = Test::using_fixture("write-body.wasm").against_empty().await;
+    let resp = Test::using_fixture("write-body.wasm")
+        .against_empty()
+        .await?;
 
     let body = body::to_bytes(resp.into_body())
         .await
@@ -23,7 +25,7 @@ async fn bodies_can_be_written_and_appended() -> TestResult {
 async fn bodies_can_be_written_and_read() -> TestResult {
     let resp = Test::using_fixture("write-and-read-body.wasm")
         .against_empty()
-        .await;
+        .await?;
     assert_eq!(resp.status(), StatusCode::OK);
     Ok(())
 }

cli/tests/integration/client_certs.rs

@@ -177,7 +177,7 @@ async fn client_certs_work() -> TestResult {
                 .body("Hello, Viceroy!")
                 .unwrap(),
         )
-        .await;
+        .await?;
     assert_eq!(resp.status(), StatusCode::OK);
     assert_eq!(
         resp.into_body().read_into_string().await?,

cli/tests/integration/common.rs

@@ -8,6 +8,7 @@ use std::{
     sync::Arc,
 };
 use tracing_subscriber::filter::EnvFilter;
+use viceroy_lib::config::UnknownImportBehavior;
 use viceroy_lib::{
     body::Body,
     config::{Dictionaries, FastlyConfig, Geolocation, ObjectStores, SecretStores},
@@ -55,6 +56,7 @@ pub struct Test {
     log_stdout: bool,
     log_stderr: bool,
     via_hyper: bool,
+    unknown_import_behavior: UnknownImportBehavior,
 }
 
 impl Test {
@@ -73,6 +75,7 @@ impl Test {
             log_stdout: false,
             log_stderr: false,
             via_hyper: false,
+            unknown_import_behavior: Default::default(),
         }
     }
 
@@ -91,6 +94,7 @@ impl Test {
             log_stdout: false,
             log_stderr: false,
             via_hyper: false,
+            unknown_import_behavior: Default::default(),
         }
     }
 
@@ -114,6 +118,15 @@ impl Test {
         self
     }
 
+    /// Use the specified [`UnknownImportBehavior`] for this test.
+    pub fn using_unknown_import_behavior(
+        mut self,
+        unknown_import_behavior: UnknownImportBehavior,
+    ) -> Self {
+        self.unknown_import_behavior = unknown_import_behavior;
+        self
+    }
+
     /// Add a backend definition to this test.
     ///
     /// The `name` is the static backend name that can be passed as, for example, the argument to
@@ -232,7 +245,7 @@ impl Test {
     pub async fn against_many(
         &self,
         mut reqs: Vec<Request<impl Into<HyperBody>>>,
-    ) -> Vec<Response<Body>> {
+    ) -> Result<Vec<Response<Body>>, Error> {
         let mut responses = Vec::with_capacity(reqs.len());
 
         // Install a tracing subscriber. We use a human-readable event formatter in tests, using a
@@ -262,8 +275,8 @@ impl Test {
             ProfilingStrategy::None,
             HashSet::new(),
             None,
-        )
-        .expect("failed to set up execution context")
+            self.unknown_import_behavior,
+        )?
         .with_backends(self.backends.backend_configs().await)
         .with_dictionaries(self.dictionaries.clone())
         .with_geolocation(self.geolocation.clone())
@@ -307,18 +320,15 @@ impl Test {
                 *req.uri_mut() = new_uri;
 
                 // Pass the request to the server via a Hyper client on the _current_ task:
-                let resp = hyper::Client::new()
-                    .request(req.map(Into::into))
-                    .await
-                    .expect("hyper client error making test request");
+                let resp = hyper::Client::new().request(req.map(Into::into)).await?;
                 responses.push(resp.map(Into::into));
             }
 
             // We're done with these test requests, so shut down the server.
             tx.send(())
                 .expect("sender error while shutting down hyper server");
             // Reap the task handle to ensure that the server did indeed shut down.
-            let _ = server_handle.await.expect("hyper server yielded an error");
+            let _ = server_handle.await?;
         } else {
             for mut req in reqs.drain(..) {
                 // We do not have to worry about an ephemeral port in the non-hyper scenario, but we
@@ -339,13 +349,26 @@ impl Test {
                     .clone()
                     .handle_request(req.map(Into::into), Ipv4Addr::LOCALHOST.into())
                     .await
-                    .map(|result| result.0)
-                    .expect("failed to handle the request");
+                    .map(|result| {
+                        match result {
+                            (resp, None) => resp,
+                            (_, Some(err)) => {
+                                // Splat the string representation of the runtime error into a synthetic
+                                // 500. This is a bit of a hack, but good enough to check for expected error
+                                // strings.
+                                let body = err.to_string();
+                                Response::builder()
+                                    .status(hyper::StatusCode::INTERNAL_SERVER_ERROR)
+                                    .body(Body::from(body.as_bytes()))
+                                    .unwrap()
+                            }
+                        }
+                    })?;
                 responses.push(resp);
             }
         }
 
-        responses
+        Ok(responses)
     }
 
     /// Pass the given request to a Viceroy execution context defined by this test.
@@ -355,15 +378,19 @@ impl Test {
     ///
     /// A `Test` can be used repeatedly against different requests. Note, however, that
     /// a fresh execution context is set up each time.
-    pub async fn against(&self, req: Request<impl Into<HyperBody>>) -> Response<Body> {
-        self.against_many(vec![req])
-            .await
+    pub async fn against(
+        &self,
+        req: Request<impl Into<HyperBody>>,
+    ) -> Result<Response<Body>, Error> {
+        Ok(self
+            .against_many(vec![req])
+            .await?
             .pop()
-            .expect("singleton back from against_many")
+            .expect("singleton back from against_many"))
     }
 
     /// Pass an empty `GET /` request through this test.
-    pub async fn against_empty(&self) -> Response<Body> {
+    pub async fn against_empty(&self) -> Result<Response<Body>, Error> {
         self.against(Request::get("/").body("").unwrap()).await
     }
 

cli/tests/integration/config_store_lookup.rs

@@ -18,7 +18,7 @@ async fn json_config_store_lookup_works() -> TestResult {
     let resp = Test::using_fixture("config_store-lookup.wasm")
         .using_fastly_toml(FASTLY_TOML)?
         .against_empty()
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::OK);
     assert!(to_bytes(resp.into_body())
@@ -49,7 +49,7 @@ async fn inline_toml_config_store_lookup_works() -> TestResult {
     let resp = Test::using_fixture("config_store-lookup.wasm")
         .using_fastly_toml(FASTLY_TOML)?
         .against_empty()
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::OK);
     assert!(to_bytes(resp.into_body())
@@ -72,7 +72,7 @@ async fn missing_config_store_works() -> TestResult {
     let resp = Test::using_fixture("config_store-lookup.wasm")
         .using_fastly_toml(FASTLY_TOML)?
         .against_empty()
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR);
 

cli/tests/integration/dictionary_lookup.rs

@@ -18,7 +18,7 @@ async fn json_dictionary_lookup_works() -> TestResult {
     let resp = Test::using_fixture("dictionary-lookup.wasm")
         .using_fastly_toml(FASTLY_TOML)?
         .against_empty()
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::OK);
     assert!(to_bytes(resp.into_body())
@@ -49,7 +49,7 @@ async fn inline_toml_dictionary_lookup_works() -> TestResult {
     let resp = Test::using_fixture("dictionary-lookup.wasm")
         .using_fastly_toml(FASTLY_TOML)?
         .against_empty()
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::OK);
     assert!(to_bytes(resp.into_body())
@@ -72,7 +72,7 @@ async fn missing_dictionary_works() -> TestResult {
     let resp = Test::using_fixture("dictionary-lookup.wasm")
         .using_fastly_toml(FASTLY_TOML)?
         .against_empty()
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::INTERNAL_SERVER_ERROR);
 

cli/tests/integration/downstream_req.rs

@@ -11,7 +11,7 @@ async fn downstream_request_works() -> TestResult {
         .body("Hello, world!")?;
     let resp = Test::using_fixture("downstream-req.wasm")
         .against(req)
-        .await;
+        .await?;
 
     assert_eq!(resp.status(), StatusCode::OK);
     Ok(())

cli/tests/integration/env_vars.rs

@@ -2,7 +2,7 @@ use crate::common::{Test, TestResult};
 
 #[tokio::test]
 async fn env_vars_are_set() -> TestResult {
-    let resp = Test::using_fixture("env-vars.wasm").against_empty().await;
+    let resp = Test::using_fixture("env-vars.wasm").against_empty().await?;
     assert!(resp.status().is_success());
     Ok(())
 }