feat: Check types of values passed to native functions, prevent panics.

[rudo-thomas]

Previously, the native functions would panic if the type of the value wasn't right. Now, there is a bit of reflection to detect incorrect types and also to do the conversions to the types the function needs.

[CLAassistant]

All committers have signed the CLA.

[rudo-thomas]

Maybe @sh0rez could take a look? Thanks.

[rudo-thomas]

Rebased against v0.25.0.

[rudo-thomas]

Rebased against current HEAD (7345b9d7) and improved a bit.

Edit: The diff is now best viewed with changes in whitespace hidden :)

[rudo-thomas]

Just to elaborate a bit on what this PR tries to achieve.

1. Cleaner implementation of native functions: There are no type assertions and hence no possible panics on unexpected code from the user. Everything is checked at runtime, using reflection.

2. Better error messages. (Though, arguably, the difference is not as stark as it was against older versions of Tanka; newer versions use newer go-jsonnet, which no longer prints a stack trace when a native function panicks -- thanks to my PR :-D)

With native-boom.jsonnet having the following content:

// to be processed by "tk eval"
std.native('parseJson')(4)  // explodes, 4 is not a string

You'd get...

With 0.26.0:

$ tk-0.26.0 eval native-boom.jsonnet 
Error: evaluating jsonnet: RUNTIME ERROR: native function "parseJson" panicked: interface conversion: interface {} is float64, not string
	/home/r/git/inframon-k8s/native-boom.jsonnet:2:1-27	$
	During evaluation	

With this PR:

$ tk-fixed eval native-boom.jsonnet 
Error: evaluating jsonnet: RUNTIME ERROR: parseJson(): argument "json" has unexpected type
	/home/r/git/inframon-k8s/native-boom.jsonnet:2:1-27	$
	During evaluation	

A stacktrace in 0.23.1 (which was around the time when I filed this PR):

$ tk-0.23.1 eval native-boom.jsonnet
Error: evaluating jsonnet: INTERNAL ERROR: (CRASH) interface conversion: interface {} is float64, not string
goroutine 1 [running]:
runtime/debug.Stack()
	/usr/local/go/src/runtime/debug/stack.go:24 +0x65
github.com/google/go-jsonnet.(*VM).Evaluate.func1()
	/go/pkg/mod/github.com/google/[email protected]/vm.go:182 +0x45
panic({0x8b3d40, 0xc00022bce0})
	/usr/local/go/src/runtime/panic.go:884 +0x212
github.com/grafana/tanka/pkg/jsonnet/native.parseJSON.func1({0xc000233600, 0x1, 0xc0002d1210?})
	/drone/src/pkg/jsonnet/native/funcs.go:46 +0xa7
...

[Duologic]

Could this be a good fit for upstream go-jsonnet? Looks like a potential improvement for jsonnet.NativeFunction.

[rudo-thomas]

That's a good point. It'd have to be a bit more generic on the types side I guess.

I'll prepare something and ask the go-jsonnet people...