Repository: grafana/tanka
Pull request #431 (2020-11-26 21:10:42)

refactor(api): Evaluators
With inline environments, I introduced quite a bit of complexity to the code base. This is a first attempt at trying to
reduce the complexity.

I've observed this pattern in https://github.com/grafana/cortex-tools/blob/master/pkg/rules/parser.go#L27-L37

malcolmholmes: Named funcs,  much clearer.

Why though use a string and a switch? You could just pass in the function itself. The callers wouldn't notice the change from string to func, no?
sh0rez: @Duologic it is not documented at the moment, but I usually refer to `pkg/tanka` as `api` in conventional commits scope, so this PR would be `refactor(api): redurce ...`, and I'd even suggest `refactor(api): Evaluators`

Patch:
diff --git a/cmd/tk/main.go b/cmd/tk/main.go
index a15afbfc..f3f220ae 100644
--- a/cmd/tk/main.go
+++ b/cmd/tk/main.go
@@ -10,7 +10,6 @@ import (
 
 	"github.com/go-clix/cli"
 
-	"github.com/grafana/tanka/pkg/jsonnet"
 	"github.com/grafana/tanka/pkg/jsonnet/jpath"
 	"github.com/grafana/tanka/pkg/spec"
 	"github.com/grafana/tanka/pkg/spec/v1alpha1"
@@ -80,7 +79,7 @@ func setupConfiguration(baseDir string) *v1alpha1.Environment {
 			}
 		// no spec.json is found, try parsing main.jsonnet
 		case spec.ErrNoSpec:
-			config, err := tanka.EvalEnvs(baseDir, jsonnet.Opts{})
+			_, config, err := tanka.ParseEnv(baseDir, tanka.ParseOpts{Evaluator: tanka.EnvsOnlyEvaluator})
 			if err != nil {
 				switch err.(type) {
 				case tanka.ErrNoEnv:
diff --git a/pkg/tanka/evaluators.go b/pkg/tanka/evaluators.go
new file mode 100644
index 00000000..33f0bc2f
--- /dev/null
+++ b/pkg/tanka/evaluators.go
@@ -0,0 +1,84 @@
+package tanka
+
+import (
+	"fmt"
+
+	"github.com/pkg/errors"
+
+	"github.com/grafana/tanka/pkg/jsonnet"
+	"github.com/grafana/tanka/pkg/jsonnet/jpath"
+)
+
+// Evaluator signature for implementing arbitrary Jsonnet evaluators
+type Evaluator func(path string, opts jsonnet.Opts) (string, error)
+
+// DefaultEvaluator evaluates the jsonnet environment at the given file system path
+func DefaultEvaluator(path string, opts jsonnet.Opts) (string, error) {
+	entrypoint, err := jpath.Entrypoint(path)
+	if err != nil {
+		return "", err
+	}
+
+	// evaluate Jsonnet
+	var raw string
+	if opts.EvalPattern != "" {
+		evalScript := fmt.Sprintf("(import '%s').%s", entrypoint, opts.EvalPattern)
+		raw, err = jsonnet.Evaluate(entrypoint, evalScript, opts)
+		if err != nil {
+			return "", errors.Wrap(err, "evaluating jsonnet")
+		}
+	} else {
+		raw, err = jsonnet.EvaluateFile(entrypoint, opts)
+		if err != nil {
+			return "", errors.Wrap(err, "evaluating jsonnet")
+		}
+	}
+	return raw, nil
+}
+
+// EnvsOnlyEvaluator finds the Environment object (without its .data object) at
+// the given file system path intended for use by the `tk env` command
+func EnvsOnlyEvaluator(path string, opts jsonnet.Opts) (string, error) {
+	entrypoint, err := jpath.Entrypoint(path)
+	if err != nil {
+		return "", err
+	}
+
+	// Snippet to find all Environment objects and remove the .data object for faster evaluation
+	noData := `
+local noDataEnv(object) =
+  if std.isObject(object)
+  then
+    if std.objectHas(object, 'apiVersion')
+       && std.objectHas(object, 'kind')
+    then
+      if object.kind == 'Environment'
+      then object { data:: {} }
+      else {}
+    else
+      std.mapWithKey(
+        function(key, obj)
+          noDataEnv(obj),
+        object
+      )
+  else if std.isArray(object)
+  then
+    std.map(
+      function(obj)
+        noDataEnv(obj),
+      object
+    )
+  else {};
+
+noDataEnv(import '%s')
+`
+
+	// evaluate Jsonnet with noData snippet
+	var raw string
+	evalScript := fmt.Sprintf(noData, entrypoint)
+	raw, err = jsonnet.Evaluate(entrypoint, evalScript, opts)
+	if err != nil {
+		return "", errors.Wrap(err, "evaluating jsonnet")
+	}
+	return raw, nil
+}
diff --git a/pkg/tanka/parse.go b/pkg/tanka/parse.go
index 2cc87e81..c79a85ea 100644
--- a/pkg/tanka/parse.go
+++ b/pkg/tanka/parse.go
@@ -9,7 +9,6 @@ import (
 	"github.com/Masterminds/semver"
 	"github.com/pkg/errors"
 
-	"github.com/grafana/tanka/pkg/jsonnet"
 	"github.com/grafana/tanka/pkg/jsonnet/jpath"
 	"github.com/grafana/tanka/pkg/kubernetes"
 	"github.com/grafana/tanka/pkg/kubernetes/manifest"
@@ -65,7 +64,7 @@ func (p *loaded) connect() (*kubernetes.Kubernetes, error) {
 
 // load runs all processing stages described at the Processed type
 func load(path string, opts Opts) (*loaded, error) {
-	_, env, err := eval(path, opts.JsonnetOpts)
+	_, env, err := ParseEnv(path, ParseOpts{JsonnetOpts: opts.JsonnetOpts})
 	if err != nil {
 		return nil, err
 	}
@@ -93,36 +92,6 @@ func load(path string, opts Opts) (*loaded, error) {
 	}, nil
 }
 
-// eval evaluates the jsonnet environment at the given file system path
-func eval(path string, opts jsonnet.Opts) (interface{}, *v1alpha1.Environment, error) {
-	return parseEnv(
-		path,
-		opts,
-		func(path string, opts jsonnet.Opts) (string, error) {
-			entrypoint, err := jpath.Entrypoint(path)
-			if err != nil {
-				return "", err
-			}
-
-			// evaluate Jsonnet
-			var raw string
-			if opts.EvalPattern != "" {
-				evalScript := fmt.Sprintf("(import '%s').%s", entrypoint, opts.EvalPattern)
-				raw, err = jsonnet.Evaluate(entrypoint, evalScript, opts)
-				if err != nil {
-					return "", errors.Wrap(err, "evaluating jsonnet")
-				}
-			} else {
-				raw, err = jsonnet.EvaluateFile(entrypoint, opts)
-				if err != nil {
-					return "", errors.Wrap(err, "evaluating jsonnet")
-				}
-			}
-			return raw, nil
-		},
-	)
-}
-
 // parseSpec parses the `spec.json` of the environment and returns a
 // *kubernetes.Kubernetes from it
 func parseSpec(path string) (*v1alpha1.Environment, error) {
@@ -152,10 +121,9 @@ func parseSpec(path string) (*v1alpha1.Environment, error) {
 	return config, nil
 }
 
-type evaluateFunc func(path string, opts jsonnet.Opts) (string, error)
-
-// parseEnv finds the Environment object at the given file system path
-func parseEnv(path string, opts jsonnet.Opts, evalFn evaluateFunc) (interface{}, *v1alpha1.Environment, error) {
+// ParseEnv evaluates the jsonnet environment at the given file system path and
+// optionally also returns and Environment object
+func ParseEnv(path string, opts ParseOpts) (interface{}, *v1alpha1.Environment, error) {
 	specEnv, err := parseSpec(path)
 	if err != nil {
 		switch err.(type) {
@@ -171,10 +139,14 @@ func parseEnv(path string, opts jsonnet.Opts, evalFn evaluateFunc) (interface{},
 		if err != nil {
 			return nil, nil, errors.Wrap(err, "marshalling environment config")
 		}
-		opts.ExtCode.Set(spec.APIGroup+"/environment", string(jsonEnv))
+		opts.JsonnetOpts.ExtCode.Set(spec.APIGroup+"/environment", string(jsonEnv))
 	}
 
-	raw, err := evalFn(path, opts)
+	if opts.Evaluator == nil {
+		opts.Evaluator = DefaultEvaluator
+	}
+
+	raw, err := opts.Evaluator(path, opts.JsonnetOpts)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -184,7 +156,7 @@ func parseEnv(path string, opts jsonnet.Opts, evalFn evaluateFunc) (interface{},
 		return nil, nil, errors.Wrap(err, "unmarshalling data")
 	}
 
-	if opts.EvalPattern != "" {
+	if opts.JsonnetOpts.EvalPattern != "" {
 		// EvalPattern has no affinity with an environment, behave as jsonnet interpreter
 		return data, nil, nil
 	}
@@ -251,6 +223,7 @@ func checkVersion(constraint string) error {
 	return nil
 }
 
+// extractEnvironments filters out any Environment manifests
 func extractEnvironments(data interface{}) (manifest.List, error) {
 	// Scan for everything that looks like a Kubernetes object
 	extracted, err := process.Extract(data)
@@ -271,57 +244,3 @@ func extractEnvironments(data interface{}) (manifest.List, error) {
 	// Extract only object of Kind: Environment
 	return process.Filter(out, process.MustStrExps("Environment/.*")), nil
 }
-
-// EvalEnvs finds the Environment object (without its .data object) at the
-// given file system path intended for use by the `tk env` command
-func EvalEnvs(path string, opts jsonnet.Opts) (*v1alpha1.Environment, error) {
-	_, env, err := parseEnv(
-		path,
-		opts,
-		func(path string, opts jsonnet.Opts) (string, error) {
-			entrypoint, err := jpath.Entrypoint(path)
-			if err != nil {
-				return "", err
-			}
-
-			// Snippet to find all Environment objects and remove the .data object for faster evaluation
-			noData := `
-local noDataEnv(object) =
-  if std.isObject(object)
-  then
-    if std.objectHas(object, 'apiVersion')
-       && std.objectHas(object, 'kind')
-    then
-      if object.kind == 'Environment'
-      then object { data:: {} }
-      else {}
-    else
-      std.mapWithKey(
-        function(key, obj)
-          noDataEnv(obj),
-        object
-      )
-  else if std.isArray(object)
-  then
-    std.map(
-      function(obj)
-        noDataEnv(obj),
-      object
-    )
-  else {};
-
-noDataEnv(import '%s')
-`
-
-			// evaluate Jsonnet with noData snippet
-			var raw string
-			evalScript := fmt.Sprintf(noData, entrypoint)
-			raw, err = jsonnet.Evaluate(entrypoint, evalScript, opts)
-			if err != nil {
-				return "", errors.Wrap(err, "evaluating jsonnet")
-			}
-			return raw, nil
-		},
-	)
-	return env, err
-}
diff --git a/pkg/tanka/tanka.go b/pkg/tanka/tanka.go
index a5bd6cd3..4bdd1afa 100644
--- a/pkg/tanka/tanka.go
+++ b/pkg/tanka/tanka.go
@@ -9,6 +9,8 @@ import (
 	"github.com/grafana/tanka/pkg/process"
 )
 
+type JsonnetOpts = jsonnet.Opts
+
 // Opts specify general, optional properties that apply to all actions
 type Opts struct {
 	JsonnetOpts
@@ -17,4 +19,7 @@ type Opts struct {
 	Filters process.Matchers
 }
 
-type JsonnetOpts = jsonnet.Opts
+type ParseOpts struct {
+	JsonnetOpts
+	Evaluator Evaluator
+}
diff --git a/pkg/tanka/workflow.go b/pkg/tanka/workflow.go
index ab301f64..ecfa5f53 100644
--- a/pkg/tanka/workflow.go
+++ b/pkg/tanka/workflow.go
@@ -184,7 +184,7 @@ func Show(baseDir string, opts Opts) (manifest.List, error) {
 
 // Eval returns the raw evaluated Jsonnet output (without any transformations)
 func Eval(dir string, opts Opts) (raw interface{}, err error) {
-	r, _, err := eval(dir, opts.JsonnetOpts)
+	r, _, err := ParseEnv(dir, ParseOpts{JsonnetOpts: opts.JsonnetOpts})
 	switch err.(type) {
 	case ErrNoEnv, ErrMultipleEnvs:
 		return r, err


Test patch:
diff --git a/pkg/tanka/parse_test.go b/pkg/tanka/parse_test.go
index ad3d5ac5..520a484d 100644
--- a/pkg/tanka/parse_test.go
+++ b/pkg/tanka/parse_test.go
@@ -3,7 +3,6 @@ package tanka
 import (
 	"testing"
 
-	"github.com/grafana/tanka/pkg/jsonnet"
 	"github.com/grafana/tanka/pkg/spec/v1alpha1"
 	"github.com/stretchr/testify/assert"
 )
@@ -112,7 +111,7 @@ func TestEvalJsonnet(t *testing.T) {
 	}
 
 	for _, test := range cases {
-		data, env, e := eval(test.baseDir, jsonnet.Opts{})
+		data, env, e := ParseEnv(test.baseDir, ParseOpts{})
 		if data == nil {
 			assert.NoError(t, e)
 		} else if e != nil {