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

Support for user defined ModuleConfig #90

Closed
wants to merge 1 commit into from

Conversation

mymmrac
Copy link
Contributor

@mymmrac mymmrac commented Jan 9, 2025

In this PR I added support for users of the plugins to manage ModuleConfig as they wish (full control), which bypasses manifest, as in some cases there is a need to provide your own implementation of FS or custom Stdout and Stderr which right now will (may) be overridden by manifest

@mhmd-azeez
Copy link
Collaborator

mhmd-azeez commented Jan 12, 2025

Hey @mymmrac, thanks for the PR!

I am not comfortable with having the ModuleConfigBypassManifest flag which makes it harder to reason about the logic and could become hard to maintain down the line.

I think your problem will be addressed if we changed Instance to make sure we only override WithFSConfig if the user has specified AllowedPaths in their manifest. Something like this:

if len(p.manifest.AllowedPaths) > 0 {
	// NOTE: this is only necessary for guest modules because
	// host modules have the same access privileges as the host itself
	fs := wazero.NewFSConfig()
	for host, guest := range p.manifest.AllowedPaths {
		if strings.HasPrefix(host, "ro:") {
			trimmed := strings.TrimPrefix(host, "ro:")
			fs = fs.WithReadOnlyDirMount(trimmed, guest)
		} else {
			fs = fs.WithDirMount(host, guest)
		}
	}

	moduleConfig = moduleConfig.WithFSConfig(fs)
}

// NOTE: we don't want wazero to call the start function, we will initialize
// the guest runtime manually.
// See: https://github.com/extism/go-sdk/pull/1#issuecomment-1650527495
moduleConfig = moduleConfig.WithStartFunctions()

This way, you can then manually add the WithDirMount and WithReadOnlyDirMounts to your FS instance (and not rely on the manifest's AllowedPaths). Would that be an acceptable fix?

As for stdout and stderr, correct me if I am wrong, I believe it doesn't override your config unless you set the EXTISM_ENABLE_WASI_OUTPUT environment variable.

Another solution would be to add a nullable FSConfig property to PluginInstanceConfig

I am curious what @bhelx and @nilslice think

@mhmd-azeez mhmd-azeez requested review from bhelx and nilslice January 12, 2025 11:29
@mymmrac
Copy link
Contributor Author

mymmrac commented Jan 12, 2025

Hi, thanks for feedback, some thoughts:

I am not comfortable with having the ModuleConfigBypassManifest flag which makes it harder to reason about the logic and could become hard to maintain down the line.

Agree, this was also an issue for me, but at the moment I couldn't figure out another way to provide full access to ModuleConfig

I think your problem will be addressed if we changed Instance to make sure we only override WithFSConfig if the user has specified AllowedPaths in their manifest.

This can work, but still I think it only covers the issue of complex configuration logic (in this case if you provided both AllowedPaths and ModuleConfig it still will override each other, but maybe that's fine)

As for stdout and stderr, correct me if I am wrong, I believe it doesn't override your config unless you set the EXTISM_ENABLE_WASI_OUTPUT environment variable.

Correct, but still this might be an issue, let me explain: I am currently working on a library for providing granular permissions to host resources (fs, stdin/stdout, time, random, network, etc.) and for this purpose Extism fits very well I think, however there shouldn't be a way to override those permissions outside of original configuration provided by the user as it may be dangerous in some cases, that's why I think there should be a way to disable EXTISM_ENABLE_WASI_OUTPUT env

I am okay with any solution that would allow having full control over ModuleConfig (except maybe WithName and WithStartFunctions methods as those are required to be managed by Extism), I am open for suggestions and happy to help with implementation

@mhmd-azeez
Copy link
Collaborator

mhmd-azeez commented Jan 13, 2025

@mymmrac thanks for the elaboration, that makes your use case clearer. What does the API of your library look like? maybe the user can pass in their manifest to your function, which can allow you to have full control over the plugin initialization (by overriding parts of the manifest)

As for EXTISM_ENABLE_WASI_OUTPUT, the environment variable ensures that the Go SDK is consistent with the other Extism SDKs. In your library, maybe you can call os.Setenv to temporarily disable EXTISM_ENABLE_WASI_OUTPUT before initializing the plugin.

Would these suggestions address your concerns?

@mymmrac
Copy link
Contributor Author

mymmrac commented Jan 13, 2025

Hmm, yes os.Setenv may work for me, this resolves issue with env I think

Regarding the library currently (it's still in early stage of development and not even public yet) I have similar way to construct the plugin as it's done in Extism but you don't provide Manifest and PluginConfig, you provide Environment, the goal is to be able to unmarshal Environment from file or CLI args for running "plugin" directly and for lib users to be able to set more specific rules in code.

There are multiple ways to set something in Environment, so I provide multiple ways to get the result:

  • func (e *Environment) MakeModuleConfig() wazero.ModuleConfig
  • func (e *Environment) MakeRuntimeConfig() wazero.RuntimeConfig
  • func (e *Environment) MakeManifest() extism.Manifest
  • func (e *Environment) MakePluginConfig() extism.PluginConfig

This still I think will require some adjustments (WIP)

// Environment configures the behavior of WASM module.
//
// Note: Many environment configurations override each other, for example providing ModuleConfig will override all
// other configurations related to envs, args, FS, etc. Be aware that you may end up with unexpected behavior.
//
// TODO: Add json/yaml/toml tags
type Environment struct {
	// ==== Envs ====
	// Defaults to none.

	// Envs sets an environment variables.
	Envs []string
	// EnvsMap sets an environment variables as a map.
	EnvsMap map[string]string
	// EnvsFile sets an environment variables from a file.
	EnvsFile string
	// EnvsFromHost pass thought environment variables from the host.
	EnvsFromHost bool

	// ==== Args ====
	// Defaults to none.

	// Args assigns command-line arguments.
	Args []string
	// ArgsFile assigns command-line arguments from a file.
	ArgsFile string
	// ArgsFromHost pass thought command-line arguments from the host.
	ArgsFromHost bool

	// ==== Stdin ====
	// Defaults to always return io.EOF.

	// Stdin configures where standard input (file descriptor 0) is read.
	Stdin io.Reader
	// StdinFile configures standard input (file descriptor 0) to read from a file.
	StdinFile string
	// StdinFromHost pass thought stdin from the host.
	StdinFromHost bool

	// ==== Stdout ====
	// Defaults to io.Discard.

	// Stdout configures where standard output (file descriptor 1) is written.
	Stdout io.Writer
	// StdoutFile configures standard output (file descriptor 1) to write to a file.
	StdoutFile string
	// StdoutFromHost pass thought stdout from the host.
	StdoutFromHost bool

	// ==== Stderr ====
	// Defaults to io.Discard.

	// Stderr configures where standard error (file descriptor 2) is written.
	Stderr io.Writer
	// StderrFile configures standard error (file descriptor 2) to write to a file.
	StderrFile string
	// StderrFromHost pass thought stderr from the host.
	StderrFromHost bool

	// ==== FS ====
	// Defaults to have no file access.

	// FS configures the filesystem.
	FS fs.FS
	// FSFile configures the filesystem mount points.
	FSConfig wazero.FSConfig
	// FSDir configures the filesystem as a root directory.
	FSDir string
	// FSAllowedPaths configures the allowed filesystem paths that will be mapped in WASM module.
	FSAllowedPaths map[string]string
	// FSFromHost pass thought filesystem from the host.
	FSFromHost bool

	// TODO: Add file permissions + create/remove files

	// ==== Random Source ====
	// Defaults to a deterministic source.

	// RandSource configures a source of random bytes.
	RandSource io.Reader
	// RandSourceFile configures a source of random bytes from a file.
	RandSourceFile string
	// RandSourceFromHost pass thought random source from the host.
	RandSourceFromHost bool

	// ==== Wall Time ====
	// Defaults to always return UTC 1 January 1970.

	// WallTime returns the current unix/epoch time, seconds since midnight UTC 1 January 1970,
	// with a nanosecond fraction.
	WallTime func() (sec int64, ns int32)
	// WallTimeFromHost pass thought wall time from the host.
	WallTimeFromHost bool

	// ==== Nano Time ====
	// Defaults to always return 0.

	// NanoTime returns nanoseconds since an arbitrary start point, used to measure elapsed time.
	// This is sometimes referred to as a tick or monotonic time.
	NanoTime func() int64
	// NanoTimeFromHost pass thought nano time from the host.
	NanoTimeFromHost bool

	// ==== Nano Sleep ====
	// Defaults to always return immediately.

	// NanoSleep puts the current goroutine to sleep for at least ns nanoseconds.
	NanoSleep func(ns int64)
	// NanoSleepFromHost pass thought nano sleep from the host.
	NanoSleepFromHost bool

	// ==== Start Functions ====
	// Defaults to none.

	// StartFunctions configures the functions to call after the module is instantiated.
	StartFunctions []string

	// ==== Memory ====

	// MemoryLimitPages overrides the maximum pages allowed per memory. Defaults to 65536, allowing 4GB total memory
	// per instance if the maximum is not encoded in a Wasm binary. Max is 65536 (2^16) pages or 4GB.
	MemoryLimitPages uint32

	// MemoryCapacityFromMax eagerly allocates max memory. Defaults to false, which means minimum memory is
	// allocated and any call to grow memory results in re-allocations.
	MemoryCapacityFromMax bool

	// ==== Execution Time ====

	// MaxExecutionDuration limits the maximum execution time. Defaults to 0 (no limit).
	MaxExecutionDuration time.Duration

	// ==== Debug ====

	// DebugInfoEnabled toggles DWARF-based stack traces in the face of runtime errors. Defaults to false.
	DebugInfoEnabled bool

	// ==== Network ====

	// TODO: Add network access

	// ==== WASI ====

	// DisableWASI disables WASI Preview 1 support. Defaults to false.
	DisableWASIP1 bool

	// ==== Wazero ===

	// ModuleConfig configures the WASM module.
	ModuleConfig wazero.ModuleConfig
	// RuntimeConfig configures the WASM runtime.
	RuntimeConfig wazero.RuntimeConfig

	// ==== Extism ====

	// Manifest is the plugin manifest that can be provided instead of configuration above.
	Manifest *extism.Manifest
	// PluginConfig is the plugin configuration that can be provided instead of configuration above.
	PluginConfig *extism.PluginConfig

	// ==== Host Functions ====

	// HostFunctions host functions available to the guest WASM module.
	HostFunctions []extism.HostFunction
}

@mhmd-azeez
Copy link
Collaborator

I see, if you have control over the generated manifest (through func (e *Environment) MakeManifest() extism.Manifest), maybe you can generate something that doesn't include AllowedPaths, and we can change extism to not override FS if len(AllowedPaths) = 0

@mymmrac
Copy link
Contributor Author

mymmrac commented Jan 13, 2025

Yep, that may work for me, I can do those changes (probably today) and will create new PR for that

@mymmrac
Copy link
Contributor Author

mymmrac commented Jan 13, 2025

Closing this PR, instead please see #91 as proposed solution

@mymmrac mymmrac closed this Jan 13, 2025
@mymmrac mymmrac deleted the user-defined-module-config branch January 16, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants