Repository: grafana/tanka
Pull request #375 (2020-09-09 19:16:16)

refactor: Jsonnet parsing
4a2d018b4cbab1a78b8c3fc20f4c5f7ffe0bad36: Switches from `vm.EvaluateSnippet` to the new `vm.EvaluateFile` and `vm.EvaluateAnonymousSnippet` function classes, which were introduced in https://github.com/google/go-jsonnet/pull/447.

Our `pkg/jsonnet` now has three primary methods:
- `MakeVM(opts)`, which creates a Tanka specific VM, including import paths, native funcs, TLA, extVars, etc
- `EvaluateFile(file, opts)`, which uses above VM to render a file found by the importer
- `Evaluate(filename, snippet, opts)`, which uses above VM to render a piece of code that is not actually on the local file system, but injected from somewhere else

The latter two functions do not much on their own, but stay closer to the current api, where a consumer does not need to know about the VM at all

916b6c30ae8e1a8b8121453cfbfecf7acb27d6a3: Fixes the TLA feature. Apparently, we only collected top level arguments, but accidentally passed these as extVars to the VM. Oops

The refactoring already properly exposed this functionality, in this commit I only changed the "wiring" away from boilerplate function modifiers to "just" passing the struct, which seems to be more fool proof in the long run (as observed here)


Patch:
diff --git a/go.mod b/go.mod
index 8a06dbdc..020d88ae 100644
--- a/go.mod
+++ b/go.mod
@@ -9,7 +9,7 @@ require (
 	github.com/go-clix/cli v0.1.1
 	github.com/gobwas/glob v0.2.3
 	github.com/google/go-cmp v0.5.2-0.20200818193711-d2fcc899bdc2
-	github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785
+	github.com/google/go-jsonnet v0.16.1-0.20200908152747-b70cbd441a39
 	github.com/karrick/godirwalk v1.15.5
 	github.com/pkg/errors v0.8.1
 	github.com/posener/complete v1.2.3
diff --git a/go.sum b/go.sum
index 538a0c44..44580aba 100644
--- a/go.sum
+++ b/go.sum
@@ -37,6 +37,8 @@ github.com/google/go-cmp v0.5.2-0.20200818193711-d2fcc899bdc2 h1:CZtx9gNen+kr3Pu
 github.com/google/go-cmp v0.5.2-0.20200818193711-d2fcc899bdc2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785 h1:+dlQ7fPoeAqO0U9V+94golo/rW1/V2Pn+v8aPp3ljRM=
 github.com/google/go-jsonnet v0.15.1-0.20200331184325-4f4aa80dd785/go.mod h1:sOcuej3UW1vpPTZOr8L7RQimqai1a57bt5j22LzGZCw=
+github.com/google/go-jsonnet v0.16.1-0.20200908152747-b70cbd441a39 h1:noLRnY1ESguFGDPxXvIcESe2rG63f+ZSbSGYfVa6iHo=
+github.com/google/go-jsonnet v0.16.1-0.20200908152747-b70cbd441a39/go.mod h1:sOcuej3UW1vpPTZOr8L7RQimqai1a57bt5j22LzGZCw=
 github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
 github.com/google/gofuzz v1.1.0 h1:Hsa8mG0dQ46ij8Sl2AYJDUv1oA9/d6Vk+3LG99Oe02g=
 github.com/google/gofuzz v1.1.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg=
diff --git a/pkg/jsonnet/eval.go b/pkg/jsonnet/eval.go
index 787e5cf9..ea996616 100644
--- a/pkg/jsonnet/eval.go
+++ b/pkg/jsonnet/eval.go
@@ -1,7 +1,6 @@
 package jsonnet
 
 import (
-	"io/ioutil"
 	"path/filepath"
 
 	jsonnet "github.com/google/go-jsonnet"
@@ -15,51 +14,63 @@ import (
 // See jsonnet.With* for this.
 type Modifier func(vm *jsonnet.VM) error
 
-// EvaluateFile opens the file, reads it into memory and evaluates it afterwards (`Evaluate()`)
-func EvaluateFile(jsonnetFile string, mods ...Modifier) (string, error) {
-	bytes, err := ioutil.ReadFile(jsonnetFile)
-	if err != nil {
-		return "", err
-	}
+// InjectedCode holds data that is "late-bound" into the VM
+type InjectedCode map[string]string
 
-	jpath, _, _, err := jpath.Resolve(filepath.Dir(jsonnetFile))
-	if err != nil {
-		return "", errors.Wrap(err, "resolving jpath")
+// Set allows to set values on an InjectedCode, even when it is nil
+func (i *InjectedCode) Set(key, value string) {
+	if *i == nil {
+		*i = make(InjectedCode)
 	}
-	return Evaluate(jsonnetFile, string(bytes), jpath, mods...)
+
+	(*i)[key] = value
 }
 
-// Evaluate renders the given jsonnet into a string
-func Evaluate(filename, sonnet string, jpath []string, mods ...Modifier) (string, error) {
+// Opts are additional properties for the Jsonnet VM
+type Opts struct {
+	ExtCode     InjectedCode
+	TLACode     InjectedCode
+	ImportPaths []string
+}
+
+// MakeVM returns a Jsonnet VM with some extensions of Tanka, including:
+// - extended importer
+// - extCode and tlaCode applied
+// - native functions registered
+func MakeVM(opts Opts) *jsonnet.VM {
 	vm := jsonnet.MakeVM()
-	vm.Importer(NewExtendedImporter(jpath))
+	vm.Importer(NewExtendedImporter(opts.ImportPaths))
 
-	for _, mod := range mods {
-		if err := mod(vm); err != nil {
-			return "", err
-		}
+	for k, v := range opts.ExtCode {
+		vm.ExtCode(k, v)
+	}
+	for k, v := range opts.TLACode {
+		vm.TLACode(k, v)
 	}
 
 	for _, nf := range native.Funcs() {
 		vm.NativeFunction(nf)
 	}
 
-	return vm.EvaluateSnippet(filename, sonnet)
+	return vm
 }
 
-// WithExtCode allows to make the supplied snippet available to Jsonnet as an
-// ext var
-func WithExtCode(key, code string) Modifier {
-	return func(vm *jsonnet.VM) error {
-		vm.ExtCode(key, code)
-		return nil
+// EvaluateFile evaluates the Jsonnet code in the given file and returns the
+// result in JSON form. It disregards opts.ImportPaths in favor of automatically
+// resolving these according to the specified file.
+func EvaluateFile(jsonnetFile string, opts Opts) (string, error) {
+	jpath, _, _, err := jpath.Resolve(filepath.Dir(jsonnetFile))
+	if err != nil {
+		return "", errors.Wrap(err, "resolving import paths")
 	}
+	opts.ImportPaths = jpath
+
+	vm := MakeVM(opts)
+	return vm.EvaluateFile(jsonnetFile)
 }
 
-// WithTLA allows to set the given code as a top level argument
-func WithTLA(key, code string) Modifier {
-	return func(vm *jsonnet.VM) error {
-		vm.TLACode(key, code)
-		return nil
-	}
+// Evaluate renders the given jsonnet into a string
+func Evaluate(filename, data string, opts Opts) (string, error) {
+	vm := jsonnet.MakeVM()
+	return vm.EvaluateAnonymousSnippet(filename, data)
 }
diff --git a/pkg/tanka/parse.go b/pkg/tanka/parse.go
index 9340b84a..cb6b4915 100644
--- a/pkg/tanka/parse.go
+++ b/pkg/tanka/parse.go
@@ -56,7 +56,7 @@ func (p *loaded) connect() (*kubernetes.Kubernetes, error) {
 
 // load runs all processing stages described at the Processed type
 func load(dir string, opts *options) (*loaded, error) {
-	raw, env, err := eval(dir, opts.extCode)
+	raw, env, err := eval(dir, opts.jsonnet)
 	if err != nil {
 		return nil, err
 	}
@@ -74,7 +74,7 @@ func load(dir string, opts *options) (*loaded, error) {
 
 // eval runs all processing stages describe at the Processed type apart from
 // post-processing, thus returning the raw Jsonnet result.
-func eval(dir string, extCode map[string]string) (raw interface{}, env *v1alpha1.Config, err error) {
+func eval(dir string, opts jsonnet.Opts) (raw interface{}, env *v1alpha1.Config, err error) {
 	_, baseDir, rootDir, err := jpath.Resolve(dir)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "resolving jpath")
@@ -85,7 +85,7 @@ func eval(dir string, extCode map[string]string) (raw interface{}, env *v1alpha1
 		return nil, nil, err
 	}
 
-	raw, err = evalJsonnet(baseDir, env, extCode)
+	raw, err = evalJsonnet(baseDir, env, opts)
 	if err != nil {
 		return nil, nil, errors.Wrap(err, "evaluating jsonnet")
 	}
@@ -119,27 +119,22 @@ func parseSpec(baseDir, rootDir string) (*v1alpha1.Config, error) {
 
 // evalJsonnet evaluates the jsonnet environment at the given directory starting with
 // `main.jsonnet`
-func evalJsonnet(baseDir string, env *v1alpha1.Config, extCode map[string]string) (interface{}, error) {
+func evalJsonnet(baseDir string, env *v1alpha1.Config, opts jsonnet.Opts) (interface{}, error) {
+	// make env spec accessible from Jsonnet
 	jsonEnv, err := json.Marshal(env)
 	if err != nil {
 		return nil, errors.Wrap(err, "marshalling environment config")
 	}
+	opts.ExtCode.Set(spec.APIGroup+"/environment", string(jsonEnv))
 
-	ext := []jsonnet.Modifier{
-		jsonnet.WithExtCode(spec.APIGroup+"/environment", string(jsonEnv)),
-	}
-	for k, v := range extCode {
-		ext = append(ext, jsonnet.WithExtCode(k, v))
-	}
-
-	raw, err := jsonnet.EvaluateFile(
-		filepath.Join(baseDir, "main.jsonnet"),
-		ext...,
-	)
+	// evaluate Jsonnet
+	mainFile := filepath.Join(baseDir, "main.jsonnet")
+	raw, err := jsonnet.EvaluateFile(mainFile, opts)
 	if err != nil {
 		return nil, err
 	}
 
+	// parse result
 	var data interface{}
 	if err := json.Unmarshal([]byte(raw), &data); err != nil {
 		return nil, err
diff --git a/pkg/tanka/tanka.go b/pkg/tanka/tanka.go
index bc58e93e..16dd87b3 100644
--- a/pkg/tanka/tanka.go
+++ b/pkg/tanka/tanka.go
@@ -5,6 +5,7 @@
 package tanka
 
 import (
+	"github.com/grafana/tanka/pkg/jsonnet"
 	"github.com/grafana/tanka/pkg/kubernetes"
 	"github.com/grafana/tanka/pkg/process"
 )
@@ -20,8 +21,7 @@ func parseModifiers(mods []Modifier) *options {
 
 type options struct {
 	// `std.extVar`
-	extCode map[string]string
-	tlaCode map[string]string
+	jsonnet jsonnet.Opts
 
 	// target regular expressions to limit the working set
 	targets process.Matchers
@@ -41,13 +41,13 @@ type Modifier func(*options)
 // WithExtCode allows to pass external variables (jsonnet code) to the VM
 func WithExtCode(code map[string]string) Modifier {
 	return func(opts *options) {
-		opts.extCode = code
+		opts.jsonnet.ExtCode = code
 	}
 }
 
 func WithTLACode(code map[string]string) Modifier {
 	return func(opts *options) {
-		opts.extCode = code
+		opts.jsonnet.TLACode = code
 	}
 }
 
diff --git a/pkg/tanka/workflow.go b/pkg/tanka/workflow.go
index b0e95ce2..61cd6897 100644
--- a/pkg/tanka/workflow.go
+++ b/pkg/tanka/workflow.go
@@ -147,7 +147,7 @@ func Show(baseDir string, mods ...Modifier) (manifest.List, error) {
 func Eval(dir string, mods ...Modifier) (raw interface{}, err error) {
 	opts := parseModifiers(mods)
 
-	r, _, err := eval(dir, opts.extCode)
+	r, _, err := eval(dir, opts.jsonnet)
 	if err != nil {
 		return nil, err
 	}


Test patch:
diff --git a/pkg/jsonnet/jpath/jpath_test.go b/pkg/jsonnet/jpath/jpath_test.go
index 490e1ac8..bb872f70 100644
--- a/pkg/jsonnet/jpath/jpath_test.go
+++ b/pkg/jsonnet/jpath/jpath_test.go
@@ -12,7 +12,7 @@ import (
 )
 
 func TestResolvePrecedence(t *testing.T) {
-	s, err := jsonnet.EvaluateFile(filepath.Join("./testdata/precedence/environments/default/main.jsonnet"))
+	s, err := jsonnet.EvaluateFile(filepath.Join("./testdata/precedence/environments/default/main.jsonnet"), jsonnet.Opts{})
 	require.NoError(t, err)
 
 	want := map[string]string{
diff --git a/pkg/process/data_test.go b/pkg/process/data_test.go
index d18cdf94..5fd63bf5 100644
--- a/pkg/process/data_test.go
+++ b/pkg/process/data_test.go
@@ -3,7 +3,6 @@ package process
 import (
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"strings"
 
 	"github.com/grafana/tanka/pkg/jsonnet"
@@ -18,12 +17,12 @@ type testData struct {
 
 func loadFixture(name string) testData {
 	filename := "./testdata/td" + strings.Title(name) + ".jsonnet"
-	raw, err := ioutil.ReadFile(filename)
-	if err != nil {
-		panic(fmt.Sprint("loading fixture:", err))
-	}
 
-	data, err := jsonnet.Evaluate(filename, string(raw), []string{"./testdata"})
+	vm := jsonnet.MakeVM(jsonnet.Opts{
+		ImportPaths: []string{"./testdata"},
+	})
+
+	data, err := vm.EvaluateFile(filename)
 	if err != nil {
 		panic(fmt.Sprint("loading fixture:", err))
 	}
diff --git a/pkg/tanka/parse_test.go b/pkg/tanka/parse_test.go
index fe6014d1..2715e0e0 100644
--- a/pkg/tanka/parse_test.go
+++ b/pkg/tanka/parse_test.go
@@ -1,9 +1,11 @@
 package tanka
 
 import (
+	"testing"
+
+	"github.com/grafana/tanka/pkg/jsonnet"
 	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 	"github.com/stretchr/testify/assert"
-	"testing"
 )
 
 func TestEvalJsonnet(t *testing.T) {
@@ -33,8 +35,7 @@ func TestEvalJsonnet(t *testing.T) {
 	}
 
 	for _, test := range cases {
-		m := make(map[string]string)
-		data, e := evalJsonnet(test.baseDir, v1alpha1.New(), m)
+		data, e := evalJsonnet(test.baseDir, v1alpha1.New(), jsonnet.Opts{})
 		assert.NoError(t, e)
 		assert.Equal(t, test.expected, data)
 	}