Skip to content

Commit

Permalink
👻 Add options to customize behavior of unknown Wasm imports
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acfoltzer committed Sep 19, 2023
1 parent a3fa5dd commit 04cbccb
Show file tree
Hide file tree
Showing 37 changed files with 363 additions and 75 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
1 change: 1 addition & 0 deletions cli/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
17 changes: 15 additions & 2 deletions cli/src/opts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
//! Command line arguments.

use viceroy_lib::config::UnknownImportBehavior;

use {
clap::{Args, Parser, Subcommand, ValueEnum},
std::net::{IpAddr, Ipv4Addr},
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions cli/tests/integration/async_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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
Expand All @@ -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");

Expand Down
4 changes: 2 additions & 2 deletions cli/tests/integration/body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ 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
Expand All @@ -23,7 +23,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(())
}
2 changes: 1 addition & 1 deletion cli/tests/integration/client_certs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?,
Expand Down
59 changes: 43 additions & 16 deletions cli/tests/integration/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -55,6 +56,7 @@ pub struct Test {
log_stdout: bool,
log_stderr: bool,
via_hyper: bool,
unknown_import_behavior: UnknownImportBehavior,
}

impl Test {
Expand All @@ -73,6 +75,7 @@ impl Test {
log_stdout: false,
log_stderr: false,
via_hyper: false,
unknown_import_behavior: Default::default(),
}
}

Expand All @@ -91,6 +94,7 @@ impl Test {
log_stdout: false,
log_stderr: false,
via_hyper: false,
unknown_import_behavior: Default::default(),
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
}

Expand Down
6 changes: 3 additions & 3 deletions cli/tests/integration/config_store_lookup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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);

Expand Down
6 changes: 3 additions & 3 deletions cli/tests/integration/dictionary_lookup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion cli/tests/integration/downstream_req.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/integration/env_vars.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Loading

0 comments on commit 04cbccb

Please sign in to comment.