RobbieMcKinstry commented 2 weeks ago

About

After making small changes to the application described in this official blog post, I discovered that environment variables were not being propagated to the application, either from the parent process' environment or variables specifically defined in Unit configuration.

Suspected Cause

At a high level, I believe the issue is in the Wasm host. Wasm environment contexts start empty and have to explicitly opt-in to inheriting environment variables.

Specifically, these lines of code, reproduced below, seem to illustrate the issue:

let mut cx = WasiCtxBuilder::new();
// NB: while useful for debugging untrusted code probably
// shouldn't get raw access to stdout/stderr.
cx.inherit_stdout();
cx.inherit_stderr();

As you can see, the WasiCxtBuilder explicitly opts-in to inheriting stdout and stderr`, but it does not opt-in to inheriting environment variables. This API should be used to inherit env vars:

let mut cx = WasiCtxBuilder::new();
// NB: while useful for debugging untrusted code probably
// shouldn't get raw access to stdout/stderr.
cx.inherit_stdout();
cx.inherit_stderr();
+ cx.inherit_env();

I'm not sure if that's enough to inherit variables defined according to the environment configuration, which I found does not work. I would expect that during setup, the application configuration provided here would inject the environment section of the config file, and environment variable would have to be copied out of that config struct and into the WasiCtx here.

It looks like that's what's done later in the same method with the dirs field, stored in GlobalConfig, which is used to provide filesystem access to the Wasm component which would be otherwise unable to read from the filesystem.

Code Sample

The following code sample adapts the application presented in the aforementioned blog post. The output demonstrates that no environment variables were observed. To reproduce, initialize the repository as described in the blog post, then alter the config.js file and src/lib.rs as described below. When you visit the website, the webpage will demonstrate there aren't any environmental variables at all, neither those set by default by your system, not those defined in the Unit's environment config section.

From config.js:

{
   "listeners": {
      "*:80": {
         "pass": "applications/my-wasm-component"
      }
   },
   "applications": {
      "my-wasm-component": {
         "type": "wasm-wasi-component",
         "environment": {
            "PROVIDER_NAME": "Google Cloud",
            "Foo": "Bar"
         },
         "component": "/opt/nginx_unit.wasm"
      }
   }
}

From src/lib.rs:

use std::collections::HashMap;

use wasi::http::types::{
    Fields, IncomingRequest, OutgoingBody, OutgoingResponse, ResponseOutparam,
};

wasi::http::proxy::export!(Component);
use wasi::cli::environment::get_environment;

struct Component;

const HTML_BODY: &str = r#"<html>
    <head>
        <title>Hello from WebAssembly!</title>
    </head>
    <body>
      {{ENV_VARS}}
    </body>
</html>"#;

impl wasi::exports::http::incoming_handler::Guest for Component {
    fn handle(_request: IncomingRequest, response_out: ResponseOutparam) {
        let hdrs = Fields::new();
        let variables = format!("{:?}", get_environment());
        let mesg = String::from(HTML_BODY).replace("{{ENV_VARS}}", variables);

        let _try = hdrs.set(
            &"Content-Type".to_string(),
            &[b"text/html; charset=utf-8".to_vec()],
        );
        let _try = hdrs.set(
            &"Content-Length".to_string(),
            &[mesg.len().to_string().as_bytes().to_vec()],
        );

        let resp = OutgoingResponse::new(hdrs);
        resp.set_status_code(200).unwrap();

        let body = resp.body().unwrap();
        ResponseOutparam::set(response_out, Ok(resp));

        let out = body.write().unwrap();
        out.blocking_write_and_flush(mesg.as_bytes()).unwrap();
        drop(out);

        OutgoingBody::finish(body, None).unwrap();
    }
}

Thank you very much for your time and attention.

ac000 commented 2 weeks ago

Thanks for the detailed report!.

Indeed I do this in our original C based wasm language module but must have missed it wasn't being done in the rust one.

This will also get environment variables set in the config file. IIRC they are read and set by the "prototype" process before the "application" process(es) are fork(2)ed.

RobbieMcKinstry commented 2 weeks ago

IIRC they are read and set by the "prototype" process before the "application" process(es) are fork(2)ed.

Awesome, that makes sense to me! The presence of the "prototype" process clearly up my concern that the values wouldn't be copied from the environment config setting. 👍🏻

Thank you for your hard work on this lovely project!

ac000 commented 1 week ago

So I'm trying this out with the below patch

diff --git ./src/wasm-wasi-component/src/lib.rs ./src/wasm-wasi-component/src/lib.rs
index b0552e81..0df33914 100644
--- ./src/wasm-wasi-component/src/lib.rs
+++ ./src/wasm-wasi-component/src/lib.rs
@@ -256,6 +256,7 @@ impl GlobalState {
                 // shouldn't get raw access to stdout/stderr.
                 cx.inherit_stdout();
                 cx.inherit_stderr();
+                cx.inherit_env();
                 for dir in self.global_config.dirs.iter() {
                     let fd = Dir::open_ambient_dir(dir, ambient_authority())
                         .with_context(|| {

But the wasm-wasi-component language module doesn't build with it...

$ make wasm-wasi-component install
cargo build --release --manifest-path src/wasm-wasi-component/Cargo.toml
   Compiling wasm-wasi-component v0.1.0 (/home/andrew/src/unit/src/wasm-wasi-component)
error[E0599]: no method named `inherit_env` found for struct `wasmtime_wasi::preview2::WasiCtxBuilder` in the current scope
   --> src/lib.rs:259:20
    |
259 |                 cx.inherit_env();
    |                    ^^^^^^^^^^^ method not found in `WasiCtxBuilder`

For more information about this error, try `rustc --explain E0599`.
error: could not compile `wasm-wasi-component` (lib) due to 1 previous error
make: *** [build/Makefile:2151: src/wasm-wasi-component/target/release/libwasm_wasi_component.so] Error 101

Yet according to the docs (as you pointed to) it is a thing!

I can add an environment variable via cx.env();, but I don't know why it can't find .inherit_env()

Pinging someone who knows rust...

Cc: @avahahn

callahad commented 1 day ago

inherit_env was added to the wasmtime-wasi crate in version 20.0.0; we're still locked to 17.0.0.

This is reason enough to go ahead and work on updating our Rust deps now.

nginx / unit

BUG: Environment Variables not passed to Wasm Applications #1312

About

Suspected Cause

Code Sample