fix: --state-values-set panic: value of type interface {} is not assignable to type string (#680)

Probably since #647 helmfile has been unable to merge nested maps in environment values if they were loaded from files. This fixes it.

The relevant test is also enhanced so that no further regression like this happens.

Fixes #677
This commit is contained in:
KUOKA Yusuke 2019-06-12 13:35:04 +09:00 committed by GitHub
parent 11eda66eaa
commit 78b03e0d92
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 113 additions and 102 deletions

View File

@ -902,8 +902,8 @@ bar: "bar1"
}
func TestVisitDesiredStatesWithReleasesFiltered_StateValueOverrides(t *testing.T) {
envTmplExpr := "{{ .Values.foo }}-{{ .Values.bar }}-{{ .Values.baz }}-{{ .Values.hoge }}-{{ .Values.fuga }}-{{ .Values.a | first | pluck \"b\" | first | first | pluck \"c\" | first }}"
relTmplExpr := "\"{{`{{ .Values.foo }}-{{ .Values.bar }}-{{ .Values.baz }}-{{ .Values.hoge }}-{{ .Values.fuga }}-{{ .Values.a | first | pluck \\\"b\\\" | first | first | pluck \\\"c\\\" | first }}`}}\""
envTmplExpr := "{{ .Values.x.foo }}-{{ .Values.x.bar }}-{{ .Values.x.baz }}-{{ .Values.x.hoge }}-{{ .Values.x.fuga }}-{{ .Values.x.a | first | pluck \"b\" | first | first | pluck \"c\" | first }}"
relTmplExpr := "\"{{`{{ .Values.x.foo }}-{{ .Values.x.bar }}-{{ .Values.x.baz }}-{{ .Values.x.hoge }}-{{ .Values.x.fuga }}-{{ .Values.x.a | first | pluck \\\"b\\\" | first | first | pluck \\\"c\\\" | first }}`}}\""
testcases := []struct {
expr, env, expected string
@ -953,35 +953,35 @@ releases:
namespace: %s
`, testcase.expr, testcase.expr, testcase.expr),
"/path/to/values.yaml": `
foo: foo
bar: bar
baz: baz
hoge: hoge
fuga: fuga
a: []
x:
foo: foo
bar: bar
baz: baz
hoge: hoge
fuga: fuga
a: []
`,
"/path/to/default.yaml": `
bar: "bar_default"
baz: "baz_default"
a:
- b: []
x:
bar: "bar_default"
baz: "baz_default"
a:
- b: []
`,
"/path/to/production.yaml": `
bar: "bar_production"
baz: "baz_production"
a:
- b: []
x:
bar: "bar_production"
baz: "baz_production"
a:
- b: []
`,
"/path/to/overrides.yaml": `
baz: baz_override
hoge: hoge_override
a:
- b:
- c: C
x:
baz: baz_override
hoge: hoge_override
a:
- b:
- c: C
`,
}
@ -1001,7 +1001,7 @@ a:
Selectors: []string{},
Env: testcase.env,
ValuesFiles: []string{"overrides.yaml"},
Set: map[string]interface{}{"hoge": "hoge_set", "fuga": "fuga_set"},
Set: map[string]interface{}{"x": map[string]interface{}{"hoge": "hoge_set", "fuga": "fuga_set"}},
}, files)
err := app.VisitDesiredStatesWithReleasesFiltered(
"helmfile.yaml", collectReleases,

View File

@ -37,7 +37,7 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e
return nil, fmt.Errorf("bug: opts.CalleePath was nil: f=%s, opts=%v", f, opts)
}
storage := state.NewStorage(opts.CalleePath, ld.logger, ld.glob)
envld := state.NewEnvironmentValuesLoader(storage, ld.readFile)
envld := state.NewEnvironmentValuesLoader(storage, ld.readFile, ld.logger)
handler := state.MissingFileHandlerError
vals, err := envld.LoadEnvironmentValues(&handler, args)
if err != nil {

View File

@ -250,10 +250,7 @@ func (st *HelmState) loadValuesEntries(missingFileHandler *string, entries []int
envVals := map[string]interface{}{}
valuesEntries := append([]interface{}{}, entries...)
ld := &EnvironmentValuesLoader{
storage: st.storage(),
readFile: st.readFile,
}
ld := NewEnvironmentValuesLoader(st.storage(), st.readFile, st.logger)
var err error
envVals, err = ld.LoadEnvironmentValues(missingFileHandler, valuesEntries)
if err != nil {

View File

@ -1,71 +0,0 @@
package state
import (
"fmt"
"github.com/imdario/mergo"
"github.com/roboll/helmfile/pkg/environment"
"github.com/roboll/helmfile/pkg/maputil"
"github.com/roboll/helmfile/pkg/tmpl"
"gopkg.in/yaml.v2"
"path/filepath"
)
type EnvironmentValuesLoader struct {
storage *Storage
readFile func(string) ([]byte, error)
}
func NewEnvironmentValuesLoader(storage *Storage, readFile func(string) ([]byte, error)) *EnvironmentValuesLoader {
return &EnvironmentValuesLoader{
storage: storage,
readFile: readFile,
}
}
func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, envValues []interface{}) (map[string]interface{}, error) {
envVals := map[string]interface{}{}
for _, v := range envValues {
switch typedValue := v.(type) {
case string:
urlOrPath := typedValue
resolved, skipped, err := ld.storage.resolveFile(missingFileHandler, "environment values", urlOrPath)
if err != nil {
return nil, err
}
if skipped {
continue
}
for _, envvalFullPath := range resolved {
tmplData := EnvironmentTemplateData{environment.EmptyEnvironment, "", map[string]interface{}{}}
r := tmpl.NewFileRenderer(ld.readFile, filepath.Dir(envvalFullPath), tmplData)
bytes, err := r.RenderToBytes(envvalFullPath)
if err != nil {
return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", envvalFullPath, err)
}
m := map[string]interface{}{}
if err := yaml.Unmarshal(bytes, &m); err != nil {
return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", envvalFullPath, err)
}
if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
return nil, fmt.Errorf("failed to load \"%s\": %v", envvalFullPath, err)
}
}
case map[interface{}]interface{}:
m, err := maputil.CastKeysToStrings(typedValue)
if err != nil {
return nil, err
}
if err := mergo.Merge(&envVals, &m, mergo.WithOverride); err != nil {
return nil, fmt.Errorf("failed to merge %v: %v", typedValue, err)
}
continue
default:
return nil, fmt.Errorf("unexpected type of value: value=%v, type=%T", typedValue, typedValue)
}
}
return envVals, nil
}

View File

@ -0,0 +1,83 @@
package state
import (
"fmt"
"github.com/imdario/mergo"
"github.com/roboll/helmfile/pkg/environment"
"github.com/roboll/helmfile/pkg/maputil"
"github.com/roboll/helmfile/pkg/tmpl"
"go.uber.org/zap"
"gopkg.in/yaml.v2"
"path/filepath"
)
type EnvironmentValuesLoader struct {
storage *Storage
readFile func(string) ([]byte, error)
logger *zap.SugaredLogger
}
func NewEnvironmentValuesLoader(storage *Storage, readFile func(string) ([]byte, error), logger *zap.SugaredLogger) *EnvironmentValuesLoader {
return &EnvironmentValuesLoader{
storage: storage,
readFile: readFile,
logger: logger,
}
}
func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, valuesEntries []interface{}) (map[string]interface{}, error) {
result := map[string]interface{}{}
for _, entry := range valuesEntries {
maps := []interface{}{}
switch strOrMap := entry.(type) {
case string:
urlOrPath := strOrMap
files, skipped, err := ld.storage.resolveFile(missingFileHandler, "environment values", urlOrPath)
if err != nil {
return nil, err
}
if skipped {
continue
}
for _, f := range files {
tmplData := EnvironmentTemplateData{environment.EmptyEnvironment, "", map[string]interface{}{}}
r := tmpl.NewFileRenderer(ld.readFile, filepath.Dir(f), tmplData)
bytes, err := r.RenderToBytes(f)
if err != nil {
return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", f, err)
}
m := map[string]interface{}{}
if err := yaml.Unmarshal(bytes, &m); err != nil {
return nil, fmt.Errorf("failed to load environment values file \"%s\": %v", f, err)
}
maps = append(maps, m)
if ld.logger != nil {
ld.logger.Debugf("envvals_loader: loaded %s:%v", strOrMap, m)
}
}
case map[interface{}]interface{}:
maps = append(maps, strOrMap)
default:
return nil, fmt.Errorf("unexpected type of value: value=%v, type=%T", strOrMap, strOrMap)
}
for _, m := range maps {
// All the nested map key should be string. Otherwise we get strange errors due to that
// mergo or reflect is unable to merge map[interface{}]interface{} with map[string]interface{} or vice versa.
// See https://github.com/roboll/helmfile/issues/677
vals, err := maputil.CastKeysToStrings(m)
if err != nil {
return nil, err
}
if err := mergo.Merge(&result, &vals, mergo.WithOverride); err != nil {
return nil, fmt.Errorf("failed to merge %v: %v", m, err)
}
}
}
return result, nil
}

View File

@ -46,6 +46,8 @@ func (r *FileRenderer) RenderTemplateFileToBuffer(file string) (*bytes.Buffer, e
return r.RenderTemplateContentToBuffer(content)
}
// RenderToBytes loads the content of the file.
// If its extension is `gotmpl` it treats the content as a go template and renders it.
func (r *FileRenderer) RenderToBytes(path string) ([]byte, error) {
var yamlBytes []byte
splits := strings.Split(path, ".")