Skip to content

Commit 3e351a3

Browse files
authored
Fix runner crash: align WASI_CONFIG wire format with controller (#14)
* Fix runner crash: align WASI_CONFIG wire format with controller env: map[string]string (not []EnvVar), dirs: hostPath/guestPath (not volumeMounts). Add golden-file contract tests for both Go and Rust sides. Fixes #13 * Handle MarshalIndent errors in test * Export BuildRunnerConfig, fix test pkg & lint * Use CARGO_MANIFEST_DIR for robust golden path
1 parent b312ce8 commit 3e351a3

6 files changed

Lines changed: 188 additions & 46 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"args": ["--verbose"],
3+
"env": {"GREETING": "hello", "PORT": "8080"},
4+
"dirs": [
5+
{"hostPath": "/mnt/data", "guestPath": "/mnt/data", "readOnly": false},
6+
{"hostPath": "/mnt/ro", "guestPath": "/mnt/ro", "readOnly": true}
7+
],
8+
"network": {
9+
"allowIpNameLookup": true,
10+
"tcpConnect": ["example.com:443"]
11+
}
12+
}

pkg/reconciler/wasmmodule/wasmmodule.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func (r *Reconciler) createService(ctx context.Context, module *api.WasmModule)
134134
serviceName := module.Name
135135

136136
// Build WASI config for the runner
137-
wasiConfig, err := buildRunnerConfig(module)
137+
wasiConfig, err := BuildRunnerConfig(module)
138138
if err != nil {
139139
return nil, fmt.Errorf("failed to build runner config: %w", err)
140140
}
@@ -220,8 +220,8 @@ type RunnerNetworkConfig struct {
220220
UDPOutgoing []string `json:"udpOutgoing,omitempty"`
221221
}
222222

223-
// buildRunnerConfig builds the WASI configuration JSON for the runner.
224-
func buildRunnerConfig(wm *api.WasmModule) (string, error) {
223+
// BuildRunnerConfig builds the WASI configuration JSON for the runner.
224+
func BuildRunnerConfig(wm *api.WasmModule) (string, error) {
225225
config := RunnerWasiConfig{
226226
Args: wm.Spec.Args,
227227
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
Copyright 2026 The Knative Authors
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package wasmmodule_test
18+
19+
import (
20+
"encoding/json"
21+
"os"
22+
"testing"
23+
24+
api "github.com/cardil/knative-serving-wasm/pkg/apis/wasm/v1alpha1"
25+
"github.com/cardil/knative-serving-wasm/pkg/reconciler/wasmmodule"
26+
corev1 "k8s.io/api/core/v1"
27+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
)
29+
30+
// TestBuildRunnerConfigMatchesGolden verifies that the JSON produced by
31+
// BuildRunnerConfig matches the documented wire format that the runner parses.
32+
// If this test fails, both the golden file and the runner must be updated together.
33+
func TestBuildRunnerConfigMatchesGolden(t *testing.T) {
34+
t.Parallel()
35+
36+
trueVal := true
37+
module := &api.WasmModule{
38+
ObjectMeta: metav1.ObjectMeta{Name: "test"},
39+
Spec: api.WasmModuleSpec{
40+
Image: "example.com/img:latest",
41+
Args: []string{"--verbose"},
42+
Env: []corev1.EnvVar{
43+
{Name: "GREETING", Value: "hello"},
44+
{Name: "PORT", Value: "8080"},
45+
},
46+
VolumeMounts: []corev1.VolumeMount{
47+
{Name: "data", MountPath: "/mnt/data", ReadOnly: false},
48+
{Name: "ro", MountPath: "/mnt/ro", ReadOnly: true},
49+
},
50+
Volumes: []corev1.Volume{
51+
{Name: "data"},
52+
{Name: "ro"},
53+
},
54+
Network: &api.NetworkSpec{
55+
Inherit: false,
56+
AllowIPNameLookup: &trueVal,
57+
TCP: &api.TCPSpec{
58+
Connect: []string{"example.com:443"},
59+
},
60+
},
61+
},
62+
}
63+
64+
got, err := wasmmodule.BuildRunnerConfig(module)
65+
if err != nil {
66+
t.Fatalf("BuildRunnerConfig() error: %v", err)
67+
}
68+
69+
goldenBytes, err := os.ReadFile("testdata/wasi_config.golden.json")
70+
if err != nil {
71+
t.Fatalf("read golden file: %v", err)
72+
}
73+
74+
assertJSONEqual(t, got, string(goldenBytes))
75+
}
76+
77+
func assertJSONEqual(t *testing.T, got, want string) {
78+
t.Helper()
79+
80+
var gotMap, wantMap any
81+
82+
if err := json.Unmarshal([]byte(got), &gotMap); err != nil {
83+
t.Fatalf("unmarshal got: %v", err)
84+
}
85+
86+
if err := json.Unmarshal([]byte(want), &wantMap); err != nil {
87+
t.Fatalf("unmarshal want: %v", err)
88+
}
89+
90+
gotJSON, err := json.MarshalIndent(gotMap, "", " ")
91+
if err != nil {
92+
t.Fatalf("marshal got normalized JSON: %v", err)
93+
}
94+
95+
wantJSON, err := json.MarshalIndent(wantMap, "", " ")
96+
if err != nil {
97+
t.Fatalf("marshal want normalized JSON: %v", err)
98+
}
99+
100+
if string(gotJSON) != string(wantJSON) {
101+
t.Errorf("BuildRunnerConfig output does not match golden.\ngot:\n%s\n\nwant:\n%s", gotJSON, wantJSON)
102+
}
103+
}

runner/src/config.rs

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,45 +23,37 @@ pub struct WasiConfig {
2323
/// OCI image containing the WASM module
2424
#[serde(default)]
2525
pub image: String,
26-
26+
2727
/// Command line arguments to pass to the WASM module
2828
#[serde(default)]
2929
pub args: Vec<String>,
30-
31-
/// Environment variables to set in the WASM module
30+
31+
/// Environment variables to set in the WASM module.
32+
/// Serialized as a JSON object (map) by the controller: {"KEY": "VALUE"}.
3233
#[serde(default)]
33-
pub env: Vec<EnvVar>,
34-
35-
/// Volume mounts to expose as WASI preopened directories
34+
pub env: HashMap<String, String>,
35+
36+
/// Directory mounts to expose as WASI preopened directories.
37+
/// Serialized as "dirs" by the controller.
3638
#[serde(default)]
37-
pub volume_mounts: Vec<VolumeMount>,
38-
39+
pub dirs: Vec<DirConfig>,
40+
3941
/// Resource requirements (memory, CPU limits)
4042
#[serde(default)]
4143
pub resources: ResourceRequirements,
42-
44+
4345
/// Network access configuration
4446
pub network: Option<NetworkSpec>,
4547
}
4648

47-
/// Environment variable configuration
48-
#[derive(Debug, Deserialize, Clone)]
49-
pub struct EnvVar {
50-
pub name: String,
51-
#[serde(default)]
52-
pub value: String,
53-
}
54-
55-
/// Volume mount configuration
49+
/// Directory mount configuration as produced by the controller.
5650
#[derive(Debug, Deserialize, Clone)]
5751
#[serde(rename_all = "camelCase")]
58-
pub struct VolumeMount {
59-
pub name: String,
60-
pub mount_path: String,
52+
pub struct DirConfig {
53+
pub host_path: String,
54+
pub guest_path: String,
6155
#[serde(default)]
6256
pub read_only: bool,
63-
#[serde(default)]
64-
pub sub_path: String,
6557
}
6658

6759
/// Resource requirements for the WASM module
@@ -137,3 +129,45 @@ impl WasiConfig {
137129
}
138130
}
139131
}
132+
133+
#[cfg(test)]
134+
mod tests {
135+
use super::*;
136+
137+
/// Contract test: the runner must parse the exact JSON produced by the controller.
138+
/// The golden file is the single source of truth for the wire format.
139+
/// Both sides must agree: update the golden file and this test together.
140+
#[test]
141+
fn test_parse_golden_wasi_config() {
142+
let golden = std::fs::read_to_string(
143+
concat!(
144+
env!("CARGO_MANIFEST_DIR"),
145+
"/../pkg/reconciler/wasmmodule/testdata/wasi_config.golden.json"
146+
),
147+
)
148+
.expect("golden file must exist");
149+
150+
let config: WasiConfig = serde_json::from_str(&golden)
151+
.expect("golden JSON must parse into WasiConfig");
152+
153+
// env is a map in the wire format
154+
assert_eq!(config.env.get("GREETING"), Some(&"hello".to_string()));
155+
assert_eq!(config.env.get("PORT"), Some(&"8080".to_string()));
156+
157+
// dirs (not volumeMounts) with hostPath/guestPath
158+
assert_eq!(config.dirs.len(), 2);
159+
assert_eq!(config.dirs[0].host_path, "/mnt/data");
160+
assert_eq!(config.dirs[0].guest_path, "/mnt/data");
161+
assert!(!config.dirs[0].read_only);
162+
assert_eq!(config.dirs[1].host_path, "/mnt/ro");
163+
assert!(config.dirs[1].read_only);
164+
165+
// args
166+
assert_eq!(config.args, vec!["--verbose"]);
167+
168+
// network
169+
let net = config.network.as_ref().expect("network must be present");
170+
assert!(net.allow_ip_name_lookup);
171+
assert_eq!(net.tcp_connect, vec!["example.com:443"]);
172+
}
173+
}

runner/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ async fn main() -> Result<()> {
2626
println!(" Image: {}", wasi_config.image);
2727
println!(" Args: {:?}", wasi_config.args);
2828
println!(" Env vars: {} entries", wasi_config.env.len());
29-
println!(" Volume mounts: {} entries", wasi_config.volume_mounts.len());
29+
println!(" Dirs: {} entries", wasi_config.dirs.len());
3030
if let Some(memory) = wasi_config.resources.get_memory() {
3131
println!(" Memory: {}", memory);
3232
}

runner/src/server.rs

Lines changed: 12 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -168,35 +168,28 @@ fn build_wasi_ctx(config: &WasiConfig) -> Result<WasiCtx> {
168168
}
169169

170170
// Add environment variables
171-
for env_var in &config.env {
172-
builder.env(&env_var.name, &env_var.value);
171+
for (key, val) in &config.env {
172+
builder.env(key, val);
173173
}
174-
175-
// Add preopened directories from volume mounts
176-
for mount in &config.volume_mounts {
174+
175+
// Add preopened directories
176+
for dir in &config.dirs {
177177
use std::path::PathBuf;
178178
use wasmtime_wasi::{DirPerms, FilePerms};
179-
180-
// Build the host path, applying subPath if specified
181-
let host_path: PathBuf = if mount.sub_path.is_empty() {
182-
PathBuf::from(&mount.mount_path)
183-
} else {
184-
PathBuf::from(&mount.mount_path).join(&mount.sub_path)
185-
};
186-
187-
let guest_path = &mount.mount_path;
188-
189-
let (dir_perms, file_perms) = if mount.read_only {
179+
180+
let host_path = PathBuf::from(&dir.host_path);
181+
let guest_path = &dir.guest_path;
182+
183+
let (dir_perms, file_perms) = if dir.read_only {
190184
(DirPerms::READ, FilePerms::READ)
191185
} else {
192186
(DirPerms::all(), FilePerms::all())
193187
};
194-
188+
195189
// Fail fast if the directory doesn't exist
196190
if !host_path.exists() {
197191
return Err(anyhow::anyhow!(
198-
"Volume mount '{}' path does not exist: {}",
199-
mount.name,
192+
"Dir mount host path does not exist: {}",
200193
host_path.display()
201194
));
202195
}

0 commit comments

Comments
 (0)