Repository: grafana/tanka
Pull request #363 (2020-08-27 13:03:55)

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.

This is extracted from #355 


Patch:
diff --git a/pkg/helmraiser/helm.go b/pkg/helmraiser/helm.go
index 32569147..488b771c 100644
--- a/pkg/helmraiser/helm.go
+++ b/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
+}
diff --git a/pkg/jsonnet/native/funcs.go b/pkg/jsonnet/native/funcs.go
index a22bffce..8dbdb463 100644
--- a/pkg/jsonnet/native/funcs.go
+++ b/pkg/jsonnet/native/funcs.go
@@ -31,7 +31,7 @@ func Funcs() []*jsonnet.NativeFunction {
 		regexMatch(),
 		regexSubst(),
 
-		helmraiser.HelmTemplate(),
+		helmraiser.NativeFunc(),
 	}
 }
 


Test patch:
diff --git a/pkg/helmraiser/helm_test.go b/pkg/helmraiser/helm_test.go
index 3b17cff8..f316fc4b 100644
--- a/pkg/helmraiser/helm_test.go
+++ b/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)
-}