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

Improve WASM runtime error messages by adding a function to retrieve errors from resourceList #978

Merged
merged 2 commits into from
Feb 21, 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
2 changes: 0 additions & 2 deletions functions/go/apply-replacements/generated/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 34 additions & 7 deletions functions/go/apply-replacements/run_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@
package main

import (
"fmt"
"syscall/js"

"github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/apply-replacements/replacements"
"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
)

func run() error {
resourceList := []byte("")

// Register js function processResourceList to the globals.
js.Global().Set("processResourceList", resourceListProcessorWrapper())
js.Global().Set("processResourceList", resourceListProcessorWrapper(&resourceList))
// Provide a second function that serves purely to also return the resourceList,
// in case of the above function failing.
js.Global().Set("processResourceListErrors", resourceListProcessorErrors(&resourceList))
// We need to ensure that the Go program is running when JavaScript calls it.
// Otherwise, it will complain the Go program has already exited.
<-make(chan bool)
Expand All @@ -37,19 +41,42 @@ func applyReplacements(input []byte) ([]byte, error) {
return fn.Run(fn.ResourceListProcessorFunc(replacements.ApplyReplacements), []byte(input))
}

func resourceListProcessorWrapper() js.Func {
// TODO: figure out a better way to surface a golang error to JS environment.
// Currently error is surfaced as a string.
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
func resourceListProcessorWrapper(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
if len(args) != 1 {
return "Invalid number of arguments passed"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps this entire function could simply either return the resourceList or empty string, always deferring to the other function to get error messages, rather than mixing those approaches. Would require an update to kptdev/kpt#3830 if we take that route.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's a good point. I think deferring to the other function to get errors makes a lot of sense. Would like to hear @droot @mortent opinion on this.

}
input := args[0].String()
applied, err := applyReplacements([]byte(input))
if err != nil {
return fmt.Errorf("unable to process resource list:", err.Error())
*resourceList = applied
return "unable to process resource list: " + err.Error()
}
*resourceList = applied
return string(applied)
})

return jsonFunc
}

// This funcion will return ALL Results with Severity error,
// meaning unrelated errors may also be included.
func resourceListProcessorErrors(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
rl, err := fn.ParseResourceList(*resourceList)
if err != nil {
return ""
}
if len(rl.Results) == 0 {
return ""
}
errorMessages := ""
for _, r := range(rl.Results) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When running in a pipeline, the function only appends Results to ResourceList.Results but not override the existing ones. So this resourceListProcessorErrors may get irrelevant error Results (from previous functions' results).

I'd suggest at least adding a comment to notify future contributors about this since it could cause some unexpected failures in certain risk conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added!

if (r.Severity == "error") {
errorMessages += r.Message
}
}
return errorMessages
})
return jsonFunc
}
6 changes: 3 additions & 3 deletions functions/go/apply-setters/applysetters/apply_setters.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ For input ApplySetters [name: env, value: "[stage, prod]"], qthe yaml node is tr
environments: # kpt-set: ${env}
- stage
- prod

*/
func (as *ApplySetters) visitMapping(object *yaml.RNode, path string) error {
return object.VisitFields(func(node *yaml.MapNode) error {
Expand Down Expand Up @@ -180,15 +179,16 @@ e.g.for input of scalar node 'nginx:1.7.1 # kpt-set: ${image}:${tag}' in the yam

apiVersion: v1
...
image: nginx:1.7.1 # kpt-set: ${image}:${tag}

image: nginx:1.7.1 # kpt-set: ${image}:${tag}

and for input ApplySetters [[name: image, value: ubuntu], [name: tag, value: 1.8.0]]
The yaml node is transformed to

apiVersion: v1
...
image: ubuntu:1.8.0 # kpt-set: ${image}:${tag}

image: ubuntu:1.8.0 # kpt-set: ${image}:${tag}
*/
func (as *ApplySetters) visitScalar(object *yaml.RNode, path string) error {
if object.IsNil() {
Expand Down
4 changes: 1 addition & 3 deletions functions/go/apply-setters/generated/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion functions/go/apply-setters/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
kyaml "sigs.k8s.io/kustomize/kyaml/yaml"
)

//nolint
// nolint
func main() {
asp := ApplySettersProcessor{}
cmd := command.Build(&asp, command.StandaloneEnabled, false)
Expand Down
46 changes: 36 additions & 10 deletions functions/go/set-labels/run_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,19 @@
package main

import (
"fmt"
"syscall/js"

"github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/set-labels/setlabels"
"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
)

func run() error {
// Register js function processResourceList to the globals.
js.Global().Set("processResourceList", resourceListProcessorWrapper())
resourceList := []byte("")

js.Global().Set("processResourceList", resourceListProcessorWrapper(&resourceList))
// Provide a second function that serves purely to also return the resourceList,
// in case of the above function failing.
js.Global().Set("processResourceListErrors", resourceListProcessorErrors(&resourceList))
// We need to ensure that the Go program is running when JavaScript calls it.
// Otherwise, it will complain the Go program has already exited.
<-make(chan bool)
Expand All @@ -38,19 +41,42 @@ func transformLabels(input []byte) ([]byte, error) {
return fn.Run(runner, []byte(input))
}

func resourceListProcessorWrapper() js.Func {
// TODO: figure out a better way to surface a golang error to JS environment.
// Currently error is surfaced as a string.
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
// This funcion will return ALL Results with Severity error,
// meaning unrelated errors may also be included.
func resourceListProcessorWrapper(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
if len(args) != 1 {
return "Invalid number of arguments passed"
}
input := args[0].String()
transformed, err := transformLabels([]byte(input))
applied, err := transformLabels([]byte(input))
if err != nil {
*resourceList = applied
return "unable to process resource list: " + err.Error()
}
*resourceList = applied
return string(applied)
})

return jsonFunc
}

func resourceListProcessorErrors(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
rl, err := fn.ParseResourceList(*resourceList)
if err != nil {
return fmt.Errorf("unable to process resource list:", err.Error())
return ""
}
if len(rl.Results) == 0 {
return ""
}
errorMessages := ""
for _, r := range(rl.Results) {
if (r.Severity == "error") {
errorMessages += r.Message
}
}
return string(transformed)
return errorMessages
})
return jsonFunc
}
45 changes: 36 additions & 9 deletions functions/go/set-namespace/run_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,20 @@
package main

import (
"fmt"
"syscall/js"

"github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/set-namespace/transformer"
"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
)

func run() error {
resourceList := []byte("")

// Register js function processResourceList to the globals.
js.Global().Set("processResourceList", resourceListProcessorWrapper())
js.Global().Set("processResourceList", resourceListProcessorWrapper(&resourceList))
// Provide a second function that serves purely to also return the resourceList,
// in case of the above function failing.
js.Global().Set("processResourceListErrors", resourceListProcessorErrors(&resourceList))
// We need to ensure that the Go program is running when JavaScript calls it.
// Otherwise, it will complain the Go program has already exited.
<-make(chan bool)
Expand All @@ -37,19 +41,42 @@ func transformNamespace(input []byte) ([]byte, error) {
return fn.Run(fn.ResourceListProcessorFunc(transformer.Run), []byte(input))
}

func resourceListProcessorWrapper() js.Func {
// TODO: figure out a better way to surface a golang error to JS environment.
// Currently error is surfaced as a string.
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
// This funcion will return ALL Results with Severity error,
// meaning unrelated errors may also be included.
func resourceListProcessorWrapper(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
if len(args) != 1 {
return "Invalid number of arguments passed"
}
input := args[0].String()
transformed, err := transformNamespace([]byte(input))
applied, err := transformNamespace([]byte(input))
if err != nil {
*resourceList = applied
return "unable to process resource list: " + err.Error()
}
*resourceList = applied
return string(applied)
})

return jsonFunc
}

func resourceListProcessorErrors(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
rl, err := fn.ParseResourceList(*resourceList)
if err != nil {
return fmt.Errorf("unable to process resource list:", err.Error())
return ""
}
if len(rl.Results) == 0 {
return ""
}
errorMessages := ""
for _, r := range(rl.Results) {
if (r.Severity == "error") {
errorMessages += r.Message
}
}
return string(transformed)
return errorMessages
})
return jsonFunc
}
43 changes: 36 additions & 7 deletions functions/go/starlark/run_js.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,22 @@
package main

import (
"fmt"
"syscall/js"

"github.com/GoogleContainerTools/kpt-functions-catalog/functions/go/starlark/starlark"
"github.com/GoogleContainerTools/kpt-functions-sdk/go/fn"
)

func run() error {
resourceList := []byte("")

// Register js function processResourceList to the globals.
js.Global().Set("processResourceList", resourceListProcessorWrapper())
js.Global().Set("processResourceList", resourceListProcessorWrapper(&resourceList))
// Provide a second function that serves purely to also return the resourceList,
// in case of the above function failing.
js.Global().Set("processResourceListErrors", resourceListProcessorErrors(&resourceList))


// We need to ensure that the Go program is running when JavaScript calls it.
// Otherwise, it will complain the Go program has already exited.
<-make(chan bool)
Expand All @@ -37,19 +43,42 @@ func executeStarlark(input []byte) ([]byte, error) {
return fn.Run(fn.ResourceListProcessorFunc(starlark.Process), []byte(input))
}

func resourceListProcessorWrapper() js.Func {
// TODO: figure out a better way to surface a golang error to JS environment.
// Currently error is surfaced as a string.
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
func resourceListProcessorWrapper(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
if len(args) != 1 {
return "Invalid number of arguments passed"
}
input := args[0].String()
applied, err := executeStarlark([]byte(input))
if err != nil {
return fmt.Errorf("unable to process resource list:", err.Error())
*resourceList = applied
return "unable to process resource list: " + err.Error()
}
*resourceList = applied
return string(applied)
})

return jsonFunc
}

// This funcion will return ALL Results with Severity error,
// meaning unrelated errors may also be included.
func resourceListProcessorErrors(resourceList *[]byte) js.Func {
jsonFunc := js.FuncOf(func(this js.Value, args []js.Value) any {
rl, err := fn.ParseResourceList(*resourceList)
if err != nil {
return ""
}
if len(rl.Results) == 0 {
return ""
}
errorMessages := ""
for _, r := range(rl.Results) {
if (r.Severity == "error") {
errorMessages += r.Message
}
}
return errorMessages
})
return jsonFunc
}