refactor(helmraiser): Abstract template code #363
Conversation
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.
| 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") | ||
| } |
There was a problem hiding this comment.
This could all be wrapped in the Helm.run() function, I would even think of returning func (c *Cmd) Output() ([]byte, error) for this.
There was a problem hiding this comment.
(bit awkward that github diff included the normalizeName, just ignore that)
There was a problem hiding this comment.
Reusing the same pattern that proved working well in pkg/kubernetes/client:
tanka/pkg/kubernetes/client/exec.go
Line 21 in 3ff6c70
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.
| 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 | ||
|
|
There was a problem hiding this comment.
If this replaces parseYamlToMap, could you move it into a function and add a unit test for it?
There was a problem hiding this comment.
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.
Abstracts the code that calls
helm templateinto a struct method, soit 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