Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: manifest improvements #42

Merged
merged 2 commits into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 76 additions & 11 deletions extism.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/sha256"
_ "embed"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -148,6 +149,16 @@ type WasmUrl struct {
Method string `json:"method,omitempty"`
}

type concreteWasm struct {
Data []byte `json:"data,omitempty"`
Path string `json:"path,omitempty"`
Url string `json:"url,omitempty"`
Headers map[string]string `json:"headers,omitempty"`
Method string `json:"method,omitempty"`
Hash string `json:"hash,omitempty"`
Name string `json:"name,omitempty"`
}

func (d WasmData) ToWasmData(ctx context.Context) (WasmData, error) {
return d, nil
}
Expand Down Expand Up @@ -217,6 +228,52 @@ type Manifest struct {
Timeout uint64 `json:"timeout_ms,omitempty"`
}

type concreteManifest struct {
Copy link
Member

@nilslice nilslice Nov 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this, but we could potentially embed the Manifest struct into this one (with no field name) and add the Wasm []concreteWasm field to it.

This way they stay in sync and we avoid duplication. Totally may not work though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did try that but got a weird stack overflow error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha hmm ok, we can always change it later if we want since it's all internal. not worth spending much time now just to avoid a tiny bit of tech debt 😅

Wasm []concreteWasm `json:"wasm"`
Memory struct {
MaxPages uint32 `json:"max_pages,omitempty"`
} `json:"memory,omitempty"`
Config map[string]string `json:"config,omitempty"`
AllowedHosts []string `json:"allowed_hosts,omitempty"`
AllowedPaths map[string]string `json:"allowed_paths,omitempty"`
Timeout uint64 `json:"timeout_ms,omitempty"`
}

func (m *Manifest) UnmarshalJSON(data []byte) error {
tmp := concreteManifest{}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I think this is where the stack overflow happens since UnmarshalJSON gets called recursively when it encounters the embedded type

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(but I don't think there's a way around it?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, other than splitting all the Manifest fields out into another struct (which ruins the usability from Go) there doesn't seem to be a good way to re-use them. I will sleep on it though.

err := json.Unmarshal(data, &tmp)
if err != nil {
return err
}

m.Memory = tmp.Memory
m.Config = tmp.Config
m.AllowedHosts = tmp.AllowedHosts
m.AllowedPaths = tmp.AllowedPaths
m.Timeout = tmp.Timeout
if m.Wasm == nil {
m.Wasm = []Wasm{}
}
for _, w := range tmp.Wasm {
if len(w.Data) > 0 {
m.Wasm = append(m.Wasm, WasmData{Data: w.Data, Hash: w.Hash, Name: w.Name})
} else if len(w.Path) > 0 {
m.Wasm = append(m.Wasm, WasmFile{Path: w.Path, Hash: w.Hash, Name: w.Name})
} else if len(w.Url) > 0 {
m.Wasm = append(m.Wasm, WasmUrl{
Url: w.Url,
Headers: w.Headers,
Method: w.Method,
Hash: w.Hash,
Name: w.Name,
})
} else {
return errors.New("Invalid Wasm entry")
}
}
return nil
}

// Close closes the plugin by freeing the underlying resources.
func (p *Plugin) Close() error {
return p.Runtime.Wazero.Close(p.Runtime.ctx)
Expand All @@ -236,6 +293,11 @@ func NewPlugin(
rconfig = config.RuntimeConfig
}

// Make sure function calls are cancelled if the context is cancelled
if manifest.Timeout > 0 {
rconfig = rconfig.WithCloseOnContextDone(true)
}

if manifest.Memory.MaxPages > 0 {
rconfig = rconfig.WithMemoryLimitPages(manifest.Memory.MaxPages)
}
Expand Down Expand Up @@ -303,16 +365,27 @@ func NewPlugin(
// See: https://github.com/extism/go-sdk/pull/1#issuecomment-1650527495
moduleConfig = moduleConfig.WithStartFunctions().WithFSConfig(fs)

for _, wasm := range manifest.Wasm {
// Try to find the main module:
// - There is always one main module
// - If a Wasm value has the Name field set to "main" then use that module
// - If there is only one module in the manifest then that is the main module by default
// - Otherwise the last module listed is the main module

for i, wasm := range manifest.Wasm {
data, err := wasm.ToWasmData(ctx)
if err != nil {
return nil, err
}

_, mainExists := modules["main"]
if data.Name == "" || i == len(manifest.Wasm)-1 && !mainExists {
data.Name = "main"
}

_, okh := hostModules[data.Name]
_, okm := modules[data.Name]

if data.Name == "env" || okh || okm {
if data.Name == "extism:host/env" || okh || okm {
return nil, fmt.Errorf("Module name collision: '%s'", data.Name)
}

Expand All @@ -326,27 +399,19 @@ func NewPlugin(
m, err := c.Wazero.InstantiateWithConfig(c.ctx, data.Data, moduleConfig.WithName(data.Name))
if err != nil {
return nil, err
} else if count > 1 && m.Name() == "" {
return nil, fmt.Errorf("Module name can't be empty if manifest contains multiple modules.")
}

modules[data.Name] = m
}

// Try to find the main module:
// - There is always one main module
// - If a Wasm value has the Name field set to "main" then use that module
// - If there is only one module in the manifest then that is the main module by default
// - Otherwise the last module listed is the main module

logLevel := LogLevelWarn
if config.LogLevel != logLevelUnset {
logLevel = config.LogLevel
}

i := 0
for _, m := range modules {
if m.Name() == "main" || i == len(modules)-1 {
if m.Name() == "main" {
p := &Plugin{
Runtime: &c,
Modules: modules,
Expand Down
41 changes: 38 additions & 3 deletions extism_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,8 @@ func TestTimeout(t *testing.T) {
manifest.Config["duration"] = "3" // sleep for 3 seconds

config := PluginConfig{
ModuleConfig: wazero.NewModuleConfig().WithSysWalltime(),
EnableWasi: true,
RuntimeConfig: wazero.NewRuntimeConfig().WithCloseOnContextDone(true),
ModuleConfig: wazero.NewModuleConfig().WithSysWalltime(),
EnableWasi: true,
}

plugin, err := NewPlugin(context.Background(), manifest, config, []HostFunction{})
Expand Down Expand Up @@ -667,6 +666,42 @@ func TestHelloHaskell(t *testing.T) {
}
}

func TestJsonManifest(t *testing.T) {
m := `
{
"wasm": [
{
"path": "wasm/sleep.wasm"
}
],
"memory": {
"max_pages": 100
},
"config": {
"key1": "value1",
"key2": "value2",
"duration": "3"
},
"timeout_ms": 100
}
`

manifest := Manifest{}
err := manifest.UnmarshalJSON([]byte(m))
if err != nil {
t.Error(err)
}

if plugin, ok := plugin(t, manifest); ok {
defer plugin.Close()

exit, _, err := plugin.Call("run_test", []byte{})

assert.Equal(t, sys.ExitCodeDeadlineExceeded, exit, "Exit code must be `sys.ExitCodeDeadlineExceeded`")
assert.Equal(t, "module closed with context deadline exceeded", err.Error())
}
}

func BenchmarkNoop(b *testing.B) {
ctx := context.Background()
cache := wazero.NewCompilationCache()
Expand Down