refactor(helmraiser): Abstract template code (#363)

* refactor(helmraiser): Abstract template code

Abstracts the code that calls `helm template` into a struct method, so
it can be used beyond the Jsonnet interface.
This enables better testing, is more readable and also leverages more of
the existing code in this project.

* style: s/helm.run/helm.cmd

pkg/helmraiser/helm.go

@@ -12,16 +12,37 @@ import (
 
 	jsonnet "github.com/google/go-jsonnet"
 	"github.com/google/go-jsonnet/ast"
+	"github.com/grafana/tanka/pkg/kubernetes/manifest"
 	"github.com/pkg/errors"
 	yaml "gopkg.in/yaml.v3"
 )
 
-type HelmConf struct {
+// Helm provides actions on Helm charts.
+type Helm struct{}
+
+func (h Helm) cmd(action string, args ...string) *exec.Cmd {
+	argv := []string{action}
+	argv = append(argv, args...)
+
+	return helmCmd(argv...)
+}
+
+func helmCmd(args ...string) *exec.Cmd {
+	binary := "helm"
+	if env := os.Getenv("TANKA_HELM_PATH"); env != "" {
+		binary = env
+	}
+	return exec.Command(binary, args...)
+}
+
+// TemplateOpts defines additional parameters that can be passed to the
+// Helm.Template action
+type TemplateOpts struct {
 	Values map[string]interface{}
 	Flags  []string
 }
 
-func confToArgs(conf HelmConf) ([]string, []string, error) {
+func confToArgs(conf TemplateOpts) ([]string, []string, error) {
 	var args []string
 	var tempFiles []string
 
@@ -48,114 +69,98 @@ func confToArgs(conf HelmConf) ([]string, []string, error) {
 	// append custom flags to args
 	args = append(args, conf.Flags...)
 
-	if len(args) == 0 {
-		args = nil
-	}
-
 	return args, tempFiles, nil
 }
 
-func parseYamlToMap(yamlFile []byte) (map[string]interface{}, error) {
-	files := make(map[string]interface{})
-	d := yaml.NewDecoder(bytes.NewReader(yamlFile))
-	for {
-		var doc, jsonDoc interface{}
-		if err := d.Decode(&doc); err != nil {
-			if err == io.EOF {
-				break
-			}
-			return nil, errors.Wrap(err, "parsing manifests")
-		}
-
-		jsonRaw, err := json.Marshal(doc)
-		if err != nil {
-			return nil, errors.Wrap(err, "marshaling mainfests")
-		}
+// Template expands a Helm Chart into a regular manifest.List using the `helm
+// template` command
+func (h Helm) Template(name, chart string, opts TemplateOpts) (manifest.List, error) {
+	confArgs, tmpFiles, err := confToArgs(opts)
+	if err != nil {
+		return nil, err
+	}
+	for _, f := range tmpFiles {
+		defer os.Remove(f)
+	}
 
-		if err := json.Unmarshal(jsonRaw, &jsonDoc); err != nil {
-			return nil, errors.Wrap(err, "unmarshaling manifests")
-		}
+	args := []string{name, chart}
+	args = append(args, confArgs...)
 
-		// Unmarshal name and kind
-		kindName := struct {
-			Kind     string `json:"kind"`
-			Metadata struct {
-				Name string `json:"name"`
-			} `json:"metadata"`
-		}{}
-		if err := json.Unmarshal(jsonRaw, &kindName); err != nil {
-			return nil, errors.Wrap(err, "subtracting kind/name through unmarshaling")
-		}
+	cmd := h.cmd("template", args...)
+	var buf bytes.Buffer
+	cmd.Stdout = &buf
+	cmd.Stderr = os.Stderr
 
-		// snake_case string
-		normalizeName := func(s string) string {
-			s = strings.ReplaceAll(s, "-", "_")
-			s = strings.ReplaceAll(s, ":", "_")
-			s = strings.ToLower(s)
-			return s
-		}
+	if err := cmd.Run(); err != nil {
+		return nil, errors.Wrap(err, "Expanding Helm Chart")
+	}
 
-		// create a map of resources for ease of use in jsonnet
-		name := normalizeName(fmt.Sprintf("%s_%s", kindName.Metadata.Name, kindName.Kind))
-		if jsonDoc != nil {
-			files[name] = jsonDoc
+	var list manifest.List
+	d := yaml.NewDecoder(&buf)
+	for {
+		var m manifest.Manifest
+		if err := d.Decode(&m); err != nil {
+			if err == io.EOF {
+				break
+			}
+			return nil, errors.Wrap(err, "Parsing Helm output")
 		}
+		list = append(list, m)
 	}
-	return files, nil
+
+	return list, nil
 }
 
-// helmTemplate wraps and runs `helm template`
-// returns the generated manifests in a map
-func HelmTemplate() *jsonnet.NativeFunction {
+// NativeFunc returns a jsonnet native function that provides the same
+// functionality as `Helm.Template` of this package
+func NativeFunc() *jsonnet.NativeFunction {
 	return &jsonnet.NativeFunction{
 		Name: "helmTemplate",
 		// Lines up with `helm template [NAME] [CHART] [flags]` except 'conf' is a bit more elaborate
 		Params: ast.Identifiers{"name", "chart", "conf"},
 		Func: func(data []interface{}) (interface{}, error) {
-			name, chart := data[0].(string), data[1].(string)
+			name, ok := data[0].(string)
+			if !ok {
+				return nil, fmt.Errorf("First argument 'name' must be of 'string' type, got '%T' instead", data[0])
+			}
 
+			chart, ok := data[1].(string)
+			if !ok {
+				return nil, fmt.Errorf("Second argument 'chart' must be of 'string' type, got '%T' instead", data[1])
+			}
+
+			// TODO: validate data[2] actually follows the struct scheme
 			c, err := json.Marshal(data[2])
 			if err != nil {
 				return "", err
 			}
-			var conf HelmConf
+			var conf TemplateOpts
 			if err := json.Unmarshal(c, &conf); err != nil {
 				return "", err
 			}
 
-			// the basic arguments to make this work
-			args := []string{
-				"template",
-				name,
-				chart,
-			}
-
-			confArgs, tempFiles, err := confToArgs(conf)
+			var h Helm
+			list, err := h.Template(name, chart, conf)
 			if err != nil {
-				return "", nil
-			}
-			for _, file := range tempFiles {
-				defer os.Remove(file)
-			}
-			if confArgs != nil {
-				args = append(args, confArgs...)
+				return nil, err
 			}
 
-			helmBinary := "helm"
-			if hc := os.Getenv("TANKA_HELM_PATH"); hc != "" {
-				helmBinary = hc
-			}
+			out := make(map[string]interface{})
+			for _, m := range list {
+				name := fmt.Sprintf("%s_%s", m.Kind(), m.Metadata().Name())
+				name = normalizeName(name)
 
-			// convert the values map into a yaml file
-			cmd := exec.Command(helmBinary, args...)
-			buf := bytes.Buffer{}
-			cmd.Stdout = &buf
-			cmd.Stderr = os.Stderr
-			if err := cmd.Run(); err != nil {
-				return nil, fmt.Errorf("running 'helm %s': %w", strings.Join(args, " "), err)
+				out[name] = map[string]interface{}(m)
 			}
 
-			return parseYamlToMap(buf.Bytes())
+			return out, nil
 		},
 	}
 }
+
+func normalizeName(s string) string {
+	s = strings.ReplaceAll(s, "-", "_")
+	s = strings.ReplaceAll(s, ":", "_")
+	s = strings.ToLower(s)
+	return s
+}

pkg/helmraiser/helm_test.go

@@ -9,7 +9,7 @@ import (
 )
 
 func TestConfToArgs_noconf(t *testing.T) {
-	conf := HelmConf{}
+	conf := TemplateOpts{}
 	args, tempFiles, err := confToArgs(conf)
 	for _, file := range tempFiles {
 		defer os.Remove(file)
@@ -20,7 +20,7 @@ func TestConfToArgs_noconf(t *testing.T) {
 }
 
 func TestConfToArgs_emptyconf(t *testing.T) {
-	conf := HelmConf{
+	conf := TemplateOpts{
 		Values: map[string]interface{}{},
 		Flags:  []string{},
 	}
@@ -35,7 +35,7 @@ func TestConfToArgs_emptyconf(t *testing.T) {
 }
 
 func TestConfToArgs_flags(t *testing.T) {
-	conf := HelmConf{
+	conf := TemplateOpts{
 		Flags: []string{
 			"--version=v0.1",
 			"--random=arg",
@@ -55,7 +55,7 @@ func TestConfToArgs_flags(t *testing.T) {
 }
 
 func TestConfToArgs_values(t *testing.T) {
-	conf := HelmConf{
+	conf := TemplateOpts{
 		Values: map[string]interface{}{
 			"hasValues": "yes",
 		},
@@ -72,7 +72,7 @@ func TestConfToArgs_values(t *testing.T) {
 }
 
 func TestConfToArgs_flagsvalues(t *testing.T) {
-	conf := HelmConf{
+	conf := TemplateOpts{
 		Values: map[string]interface{}{
 			"hasValues": "yes",
 		},
@@ -94,98 +94,3 @@ func TestConfToArgs_flagsvalues(t *testing.T) {
 	}, args)
 	assert.Nil(t, err)
 }
-
-func TestParseYamlToMap_basic(t *testing.T) {
-	yamlFile := []byte(`---
-kind: testKind
-metadata:
-  name: testName`)
-	actual, err := parseYamlToMap(yamlFile)
-
-	expected := map[string]interface{}{
-		"testname_testkind": map[string]interface{}{
-			"kind": "testKind",
-			"metadata": map[string]interface{}{
-				"name": "testName",
-			},
-		},
-	}
-	assert.Equal(t, expected, actual)
-	assert.Nil(t, err)
-}
-
-func TestParseYamlToMap_dash(t *testing.T) {
-	yamlFile := []byte(`---
-kind: testKind
-metadata:
-  name: test-Name`)
-	actual, err := parseYamlToMap(yamlFile)
-
-	expected := map[string]interface{}{
-		"test_name_testkind": map[string]interface{}{
-			"kind": "testKind",
-			"metadata": map[string]interface{}{
-				"name": "test-Name",
-			},
-		},
-	}
-	assert.Equal(t, expected, actual)
-	assert.Nil(t, err)
-}
-
-func TestParseYamlToMap_colon(t *testing.T) {
-	yamlFile := []byte(`---
-kind: testKind
-metadata:
-  name: test:Name`)
-	actual, err := parseYamlToMap(yamlFile)
-
-	expected := map[string]interface{}{
-		"test_name_testkind": map[string]interface{}{
-			"kind": "testKind",
-			"metadata": map[string]interface{}{
-				"name": "test:Name",
-			},
-		},
-	}
-	assert.Equal(t, expected, actual)
-	assert.Nil(t, err)
-}
-
-func TestParseYamlToMap_empty(t *testing.T) {
-	yamlFile := []byte(`---`)
-	actual, err := parseYamlToMap(yamlFile)
-
-	expected := map[string]interface{}{}
-	assert.Equal(t, expected, actual)
-	assert.Nil(t, err)
-}
-
-func TestParseYamlToMap_multiple_files(t *testing.T) {
-	yamlFile := []byte(`---
-kind: testKind
-metadata:
-  name: testName
----
-kind: testKind
-metadata:
-  name: testName2`)
-	actual, err := parseYamlToMap(yamlFile)
-
-	expected := map[string]interface{}{
-		"testname_testkind": map[string]interface{}{
-			"kind": "testKind",
-			"metadata": map[string]interface{}{
-				"name": "testName",
-			},
-		},
-		"testname2_testkind": map[string]interface{}{
-			"kind": "testKind",
-			"metadata": map[string]interface{}{
-				"name": "testName2",
-			},
-		},
-	}
-	assert.Equal(t, expected, actual)
-	assert.Nil(t, err)
-}

pkg/jsonnet/native/funcs.go

@@ -31,7 +31,7 @@ func Funcs() []*jsonnet.NativeFunction {
 		regexMatch(),
 		regexSubst(),
 
-		helmraiser.HelmTemplate(),
+		helmraiser.NativeFunc(),
 	}
 }