Conversation
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR adds end-user documentation for running/developing/publishing WASM functions in kpt, and (in the same change set) introduces a new CEL-based condition field on Kptfile pipeline functions to enable conditional function execution.
Changes:
- Adds a comprehensive “Using WASM Functions” guide (run, eval, push/pull, Go build tags, limitations).
- Extends
kptfile.v1.Functionwith aconditionfield and evaluates it before executing a function. - Adds a CEL evaluator implementation + unit/E2E-style tests, and updates Go module dependencies for CEL.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/api/kptfile/v1/types.go | Adds Function.Condition (CEL) field to the Kptfile API. |
| internal/fnruntime/runner.go | Initializes and uses a CEL evaluator to skip function execution when condition is false. |
| internal/fnruntime/celeval.go | New CEL evaluator implementation for evaluating conditions against resource inputs. |
| internal/fnruntime/celeval_test.go | Unit tests for CEL evaluator behavior and errors. |
| internal/fnruntime/conditional_e2e_test.go | End-to-end style tests for conditional execution behavior in FunctionRunner. |
| go.mod / go.sum | Adds github.com/google/cel-go and related transitive dependencies. |
| documentation/content/en/book/04-using-functions/wasm-functions.md | New documentation page describing the WASM function workflow in kpt. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 1. Compresses the WASM file into a tar archive | ||
| 2. Creates an OCI image with `wasm/js` platform | ||
| 3. Pushes to the registry |
There was a problem hiding this comment.
Same platform string issue as above: “wasm/js” is ambiguous; prefer “OS=js, arch=wasm” (or the correct OS/arch ordering) so users can match what registries/tools display.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Security | ||
|
|
||
| WASM functions run in a sandbox: | ||
| - No network access | ||
| - No filesystem access (except input/output resources) | ||
| - Can't execute system commands |
There was a problem hiding this comment.
The “Security” section states WASM functions have no network/filesystem access and no host access by default. kpt supports both wasmtime (default) and node.js-based runtimes (selectable via KPT_FN_WASM_RUNTIME), and the node.js runtime can expose broader host capabilities to Go WASM via syscall/js. Please qualify these claims (e.g., “with wasmtime runtime…”) to avoid overstating the sandbox guarantees.
e979fbb to
1abf41f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
|
||
| What this does: | ||
| 1. Compresses the WASM file into a tar archive | ||
| 2. Creates an OCI image with `wasm/js` platform |
There was a problem hiding this comment.
The same incorrect wasm/js platform identifier is used here. It should be js/wasm to match the OCI convention of OS/Architecture, consistent with the code in pkg/wasm/client.go:122-124 where OS: "js" and Architecture: "wasm".
documentation/content/en/book/04-using-functions/wasm-functions.md
Outdated
Show resolved
Hide resolved
| Here's how to build a Go KRM function for WASM. You need two files - one for regular builds and one for WASM: | ||
|
|
||
| `main.go` (regular build): | ||
|
|
||
| ```go | ||
| //go:build !(js && wasm) | ||
|
|
||
| package main | ||
|
|
||
| import ( | ||
| "os" | ||
|
|
||
| "github.com/kptdev/krm-functions-sdk/go/fn" | ||
| ) | ||
|
|
||
| func main() { | ||
| if err := fn.AsMain(fn.ResourceListProcessorFunc(process)); err != nil { | ||
| os.Exit(1) | ||
| } | ||
| } | ||
|
|
||
| func process(rl *fn.ResourceList) (bool, error) { | ||
| for i := range rl.Items { | ||
| // Your transformation logic | ||
| rl.Items[i].SetAnnotation("processed-by", "my-fn") | ||
| } | ||
| return true, nil | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The code example has a compilation issue: main.go defines the process function with the build tag //go:build !(js && wasm), meaning it is excluded during WASM builds. However, main_js.go (with build tag //go:build js && wasm) references process on line 184 via fn.ResourceListProcessorFunc(process). Since process is not available during WASM builds, this code will fail to compile with GOOS=js GOARCH=wasm.
The process function should be placed in a third file (e.g., process.go) without any build tags, so it's available for both regular and WASM builds. This is the typical pattern used in the referenced krm-functions-catalog.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 2. Creates an OCI image with `wasm/js` platform | ||
| 3. Pushes to the registry |
There was a problem hiding this comment.
The platform format wasm/js here is inconsistent with OCI conventions. The OCI platform format is OS/Architecture, and looking at pkg/wasm/client.go:122-125, the platform is {OS: "js", Architecture: "wasm"}. This should be written as js/wasm to match the standard OCI platform format.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // TODO: copy the implementation from main.go so behavior is consistent. | ||
| return false, nil |
There was a problem hiding this comment.
This example process function in the WASM build file has a TODO comment and returns false, nil without implementing any transformation logic. A user copying this documentation example would get a WASM function that does nothing (and returns false indicating no changes). The main.go version adds an annotation, but this version doesn't.
For a documentation example, the implementation should be consistent between the two files. Either duplicate the logic (adding the annotation) here, or refactor the example to show the recommended pattern of extracting shared logic into a separate file (e.g., processor.go) with no build tags, so both builds share the same process function. The latter pattern avoids the exact problem this code demonstrates — divergent implementations in WASM and non-WASM builds.
| // TODO: copy the implementation from main.go so behavior is consistent. | |
| return false, nil | |
| changed := false | |
| const annotationKey = "example.com/wasm" | |
| const annotationValue = "processed" | |
| for _, obj := range rl.Items { | |
| if obj == nil { | |
| continue | |
| } | |
| annotations := obj.GetAnnotations() | |
| if existing, ok := annotations[annotationKey]; ok && existing == annotationValue { | |
| continue | |
| } | |
| obj.SetAnnotation(annotationKey, annotationValue) | |
| changed = true | |
| } | |
| return changed, nil |
Add comprehensive documentation for WASM function support in kpt, covering how to run, develop, and deploy WASM functions. Closes kptdev#4296 Signed-off-by: Surbhi <agarwalsurbhi1807@gmail.com>
…s.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Surbhi <agarwalsurbhi1807@gmail.com>
…s.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Surbhi <agarwalsurbhi1807@gmail.com>
5b119ee to
46f7f9a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
WASM functions are supported in kpt but there's no documentation on how to run, develop, or deploy them. This PR adds a comprehensive guide covering the complete WASM function workflow.
Motivation
Users need documentation to understand:
--allow-alpha-wasmflagkpt alpha wasm push/pullWithout this documentation, users have to dig through code or CLI help to figure out WASM support.
Changes
Added
documentation/content/en/book/04-using-functions/wasm-functions.mdcovering:fn renderandfn evalThe code examples are based on actual WASM functions in krm-functions-catalog (set-namespace, set-labels, starlark) and follow the same pattern with separate build tags for regular and WASM builds.
Fixes #4296