Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(helmraiser): Abstract template code #363

Merged
merged 2 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
163 changes: 84 additions & 79 deletions pkg/helmraiser/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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")
}
Comment on lines +90 to +96
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could all be wrapped in the Helm.run() function, I would even think of returning func (c *Cmd) Output() ([]byte, error) for this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(bit awkward that github diff included the normalizeName, just ignore that)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reusing the same pattern that proved working well in pkg/kubernetes/client:

func (k Kubectl) ctl(action string, args ...string) *exec.Cmd {

Returning the raw exec.Cmd allows consuming functions to also modify the commands environment, stdin, capture stderr if required, etc.

I think we will need this in the future here.


// 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

Comment on lines +98 to +110
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this replaces parseYamlToMap, could you move it into a function and add a unit test for it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo the implementation of yaml unmarshalling doesn't require testing here, because go-yaml is heavily unit tested itself, and unmarshalling yaml output into a map[string]interface{} pretty standard Go stuff.

What I instead would like to test here is that the output of helm correctly ends up in Jsonnet, regardless of our actual Go implementation.
I will add such a test in a subsequent pull request, which this one is the preparation for.

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
}
105 changes: 5 additions & 100 deletions pkg/helmraiser/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{},
}
Expand All @@ -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",
Expand All @@ -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",
},
Expand All @@ -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",
},
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion pkg/jsonnet/native/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func Funcs() []*jsonnet.NativeFunction {
regexMatch(),
regexSubst(),

helmraiser.HelmTemplate(),
helmraiser.NativeFunc(),
}
}

Expand Down