fix env value lost in environment values (#605)
* fix env value lost in environment values Signed-off-by: yxxhero <aiopsclub@163.com> * add more test Signed-off-by: yxxhero <aiopsclub@163.com> Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
		
							parent
							
								
									6664f01596
								
							
						
					
					
						commit
						d8cb740dcf
					
				|  | @ -53,7 +53,7 @@ func (ld *desiredStateLoader) Load(f string, opts LoadOpts) (*state.HelmState, e | ||||||
| 		storage := state.NewStorage(opts.CalleePath, ld.logger, ld.fs) | 		storage := state.NewStorage(opts.CalleePath, ld.logger, ld.fs) | ||||||
| 		envld := state.NewEnvironmentValuesLoader(storage, ld.fs, ld.logger, ld.remote) | 		envld := state.NewEnvironmentValuesLoader(storage, ld.fs, ld.logger, ld.remote) | ||||||
| 		handler := state.MissingFileHandlerError | 		handler := state.MissingFileHandlerError | ||||||
| 		vals, err := envld.LoadEnvironmentValues(&handler, args, &environment.EmptyEnvironment) | 		vals, err := envld.LoadEnvironmentValues(&handler, args, environment.New(ld.env), ld.env) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
| 			return nil, err | 			return nil, err | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
|  | @ -15,6 +15,15 @@ type Environment struct { | ||||||
| 
 | 
 | ||||||
| var EmptyEnvironment Environment | var EmptyEnvironment Environment | ||||||
| 
 | 
 | ||||||
|  | // New return Environment with default name and values
 | ||||||
|  | func New(name string) *Environment { | ||||||
|  | 	return &Environment{ | ||||||
|  | 		Name:     name, | ||||||
|  | 		Values:   map[string]interface{}{}, | ||||||
|  | 		Defaults: map[string]interface{}{}, | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  | 
 | ||||||
| func (e Environment) DeepCopy() Environment { | func (e Environment) DeepCopy() Environment { | ||||||
| 	valuesBytes, err := yaml.Marshal(e.Values) | 	valuesBytes, err := yaml.Marshal(e.Values) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
|  |  | ||||||
|  | @ -4,6 +4,7 @@ import ( | ||||||
| 	"testing" | 	"testing" | ||||||
| 
 | 
 | ||||||
| 	"github.com/google/go-cmp/cmp" | 	"github.com/google/go-cmp/cmp" | ||||||
|  | 	"github.com/stretchr/testify/require" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| // See https://github.com/roboll/helmfile/issues/1150
 | // See https://github.com/roboll/helmfile/issues/1150
 | ||||||
|  | @ -97,3 +98,10 @@ func TestMerge_OverwriteWithNilValue_Issue1154(t *testing.T) { | ||||||
| 		t.Errorf(diff) | 		t.Errorf(diff) | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
|  | 
 | ||||||
|  | func TestNew(t *testing.T) { | ||||||
|  | 	envName := "test" | ||||||
|  | 	env := New(envName) | ||||||
|  | 
 | ||||||
|  | 	require.Equal(t, envName, env.Name, "environment name should be %s, but got %s", envName, env.Name) | ||||||
|  | } | ||||||
|  |  | ||||||
|  | @ -142,7 +142,7 @@ func (c *StateCreator) LoadEnvValues(target *HelmState, env string, ctxEnv *envi | ||||||
| 		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", state.FilePath), err} | 		return nil, &StateLoadError{fmt.Sprintf("failed to read %s", state.FilePath), err} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	newDefaults, err := state.loadValuesEntries(nil, state.DefaultValues, c.remote, ctxEnv) | 	newDefaults, err := state.loadValuesEntries(nil, state.DefaultValues, c.remote, ctxEnv, env) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, err | 		return nil, err | ||||||
| 	} | 	} | ||||||
|  | @ -217,7 +217,7 @@ func (c *StateCreator) loadEnvValues(st *HelmState, name string, failOnMissingEn | ||||||
| 	envSpec, ok := st.Environments[name] | 	envSpec, ok := st.Environments[name] | ||||||
| 	if ok { | 	if ok { | ||||||
| 		var err error | 		var err error | ||||||
| 		envVals, err = st.loadValuesEntries(envSpec.MissingFileHandler, envSpec.Values, c.remote, ctxEnv) | 		envVals, err = st.loadValuesEntries(envSpec.MissingFileHandler, envSpec.Values, c.remote, ctxEnv, name) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
| 			return nil, err | 			return nil, err | ||||||
| 		} | 		} | ||||||
|  | @ -361,13 +361,13 @@ func (c *StateCreator) scatterGatherEnvSecretFiles(st *HelmState, envSecretFiles | ||||||
| 	return nil | 	return nil | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func (st *HelmState) loadValuesEntries(missingFileHandler *string, entries []interface{}, remote *remote.Remote, ctxEnv *environment.Environment) (map[string]interface{}, error) { | func (st *HelmState) loadValuesEntries(missingFileHandler *string, entries []interface{}, remote *remote.Remote, ctxEnv *environment.Environment, envName string) (map[string]interface{}, error) { | ||||||
| 	var envVals map[string]interface{} | 	var envVals map[string]interface{} | ||||||
| 
 | 
 | ||||||
| 	valuesEntries := append([]interface{}{}, entries...) | 	valuesEntries := append([]interface{}{}, entries...) | ||||||
| 	ld := NewEnvironmentValuesLoader(st.storage(), st.fs, st.logger, remote) | 	ld := NewEnvironmentValuesLoader(st.storage(), st.fs, st.logger, remote) | ||||||
| 	var err error | 	var err error | ||||||
| 	envVals, err = ld.LoadEnvironmentValues(missingFileHandler, valuesEntries, ctxEnv) | 	envVals, err = ld.LoadEnvironmentValues(missingFileHandler, valuesEntries, ctxEnv, envName) | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		return nil, err | 		return nil, err | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -34,7 +34,7 @@ func NewEnvironmentValuesLoader(storage *Storage, fs *filesystem.FileSystem, log | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, valuesEntries []interface{}, ctxEnv *environment.Environment) (map[string]interface{}, error) { | func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *string, valuesEntries []interface{}, ctxEnv *environment.Environment, envName string) (map[string]interface{}, error) { | ||||||
| 	result := map[string]interface{}{} | 	result := map[string]interface{}{} | ||||||
| 
 | 
 | ||||||
| 	for _, entry := range valuesEntries { | 	for _, entry := range valuesEntries { | ||||||
|  | @ -59,7 +59,7 @@ func (ld *EnvironmentValuesLoader) LoadEnvironmentValues(missingFileHandler *str | ||||||
| 			for _, f := range files { | 			for _, f := range files { | ||||||
| 				var env environment.Environment | 				var env environment.Environment | ||||||
| 				if ctxEnv == nil { | 				if ctxEnv == nil { | ||||||
| 					env = environment.EmptyEnvironment | 					env = *environment.New(envName) | ||||||
| 				} else { | 				} else { | ||||||
| 					env = *ctxEnv | 					env = *ctxEnv | ||||||
| 				} | 				} | ||||||
|  |  | ||||||
|  | @ -6,6 +6,7 @@ import ( | ||||||
| 
 | 
 | ||||||
| 	"github.com/google/go-cmp/cmp" | 	"github.com/google/go-cmp/cmp" | ||||||
| 
 | 
 | ||||||
|  | 	"github.com/helmfile/helmfile/pkg/environment" | ||||||
| 	ffs "github.com/helmfile/helmfile/pkg/filesystem" | 	ffs "github.com/helmfile/helmfile/pkg/filesystem" | ||||||
| 	"github.com/helmfile/helmfile/pkg/helmexec" | 	"github.com/helmfile/helmfile/pkg/helmexec" | ||||||
| 	"github.com/helmfile/helmfile/pkg/remote" | 	"github.com/helmfile/helmfile/pkg/remote" | ||||||
|  | @ -28,7 +29,7 @@ func newLoader() *EnvironmentValuesLoader { | ||||||
| func TestEnvValsLoad_SingleValuesFile(t *testing.T) { | func TestEnvValsLoad_SingleValuesFile(t *testing.T) { | ||||||
| 	l := newLoader() | 	l := newLoader() | ||||||
| 
 | 
 | ||||||
| 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.5.yaml"}, nil) | 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.5.yaml"}, nil, "") | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		t.Fatal(err) | 		t.Fatal(err) | ||||||
| 	} | 	} | ||||||
|  | @ -42,11 +43,67 @@ func TestEnvValsLoad_SingleValuesFile(t *testing.T) { | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | func TestEnvValsLoad_EnvironmentNameFile(t *testing.T) { | ||||||
|  | 	l := newLoader() | ||||||
|  | 
 | ||||||
|  | 	expected := map[string]interface{}{ | ||||||
|  | 		"envName": "test", | ||||||
|  | 	} | ||||||
|  | 	emptyExpected := map[string]interface{}{ | ||||||
|  | 		"envName": nil, | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	tests := []struct { | ||||||
|  | 		name     string | ||||||
|  | 		env      *environment.Environment | ||||||
|  | 		envName  string | ||||||
|  | 		expected map[string]interface{} | ||||||
|  | 	}{ | ||||||
|  | 		{ | ||||||
|  | 			name:     "env is nil but envName is not", | ||||||
|  | 			env:      nil, | ||||||
|  | 			envName:  "test", | ||||||
|  | 			expected: expected, | ||||||
|  | 		}, | ||||||
|  | 		{ | ||||||
|  | 			name:     "envName is emplte but env is not", | ||||||
|  | 			env:      environment.New("test"), | ||||||
|  | 			envName:  "", | ||||||
|  | 			expected: expected, | ||||||
|  | 		}, | ||||||
|  | 		{ | ||||||
|  | 			name:     "envName and env is not nil", | ||||||
|  | 			env:      environment.New("test"), | ||||||
|  | 			envName:  "test", | ||||||
|  | 			expected: expected, | ||||||
|  | 		}, | ||||||
|  | 		{ | ||||||
|  | 			name:     "envName and env is nil", | ||||||
|  | 			env:      nil, | ||||||
|  | 			envName:  "", | ||||||
|  | 			expected: emptyExpected, | ||||||
|  | 		}, | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	for _, tt := range tests { | ||||||
|  | 		t.Run(tt.name, func(t *testing.T) { | ||||||
|  | 			actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.6.yaml.gotmpl"}, tt.env, tt.envName) | ||||||
|  | 			if err != nil { | ||||||
|  | 				t.Fatal(err) | ||||||
|  | 			} | ||||||
|  | 
 | ||||||
|  | 			if diff := cmp.Diff(tt.expected, actual); diff != "" { | ||||||
|  | 				t.Errorf(diff) | ||||||
|  | 			} | ||||||
|  | 		}) | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  | 
 | ||||||
| // Fetch Environment values from remote
 | // Fetch Environment values from remote
 | ||||||
| func TestEnvValsLoad_SingleValuesFileRemote(t *testing.T) { | func TestEnvValsLoad_SingleValuesFileRemote(t *testing.T) { | ||||||
| 	l := newLoader() | 	l := newLoader() | ||||||
| 
 | 
 | ||||||
| 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"git::https://github.com/helm/helm.git@cmd/helm/testdata/output/values.yaml?ref=v3.8.0"}, nil) | 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"git::https://github.com/helm/helm.git@cmd/helm/testdata/output/values.yaml?ref=v3.8.0"}, nil, "") | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		t.Fatal(err) | 		t.Fatal(err) | ||||||
| 	} | 	} | ||||||
|  | @ -64,7 +121,7 @@ func TestEnvValsLoad_SingleValuesFileRemote(t *testing.T) { | ||||||
| func TestEnvValsLoad_OverwriteNilValue_Issue1150(t *testing.T) { | func TestEnvValsLoad_OverwriteNilValue_Issue1150(t *testing.T) { | ||||||
| 	l := newLoader() | 	l := newLoader() | ||||||
| 
 | 
 | ||||||
| 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.1.yaml", "testdata/values.2.yaml"}, nil) | 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.1.yaml", "testdata/values.2.yaml"}, nil, "") | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		t.Fatal(err) | 		t.Fatal(err) | ||||||
| 	} | 	} | ||||||
|  | @ -86,7 +143,7 @@ func TestEnvValsLoad_OverwriteNilValue_Issue1150(t *testing.T) { | ||||||
| func TestEnvValsLoad_OverwriteWithNilValue_Issue1154(t *testing.T) { | func TestEnvValsLoad_OverwriteWithNilValue_Issue1154(t *testing.T) { | ||||||
| 	l := newLoader() | 	l := newLoader() | ||||||
| 
 | 
 | ||||||
| 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.3.yaml", "testdata/values.4.yaml"}, nil) | 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/values.3.yaml", "testdata/values.4.yaml"}, nil, "") | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		t.Fatal(err) | 		t.Fatal(err) | ||||||
| 	} | 	} | ||||||
|  | @ -109,7 +166,7 @@ func TestEnvValsLoad_OverwriteWithNilValue_Issue1154(t *testing.T) { | ||||||
| func TestEnvValsLoad_OverwriteEmptyValue_Issue1168(t *testing.T) { | func TestEnvValsLoad_OverwriteEmptyValue_Issue1168(t *testing.T) { | ||||||
| 	l := newLoader() | 	l := newLoader() | ||||||
| 
 | 
 | ||||||
| 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/issues/1168/addons.yaml", "testdata/issues/1168/addons2.yaml"}, nil) | 	actual, err := l.LoadEnvironmentValues(nil, []interface{}{"testdata/issues/1168/addons.yaml", "testdata/issues/1168/addons2.yaml"}, nil, "") | ||||||
| 	if err != nil { | 	if err != nil { | ||||||
| 		t.Fatal(err) | 		t.Fatal(err) | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | @ -0,0 +1 @@ | ||||||
|  | envName: {{ .Environment.Name }} | ||||||
|  | @ -1,6 +1,3 @@ | ||||||
| localChartRepoServer: |  | ||||||
|   enabled: true |  | ||||||
|   port: 18083 |  | ||||||
| chartifyTempDir: envs_releases_within_same_yaml_part | chartifyTempDir: envs_releases_within_same_yaml_part | ||||||
| helmfileArgs: | helmfileArgs: | ||||||
| - --environment | - --environment | ||||||
|  |  | ||||||
|  | @ -0,0 +1,5 @@ | ||||||
|  | chartifyTempDir: environments_values_gotmpl_with_environment_name | ||||||
|  | helmfileArgs: | ||||||
|  | - --environment | ||||||
|  | - test | ||||||
|  | - template | ||||||
|  | @ -0,0 +1,12 @@ | ||||||
|  | environments: | ||||||
|  |   test: | ||||||
|  |     values: | ||||||
|  |     - test.yaml.gotmpl | ||||||
|  | --- | ||||||
|  | releases: | ||||||
|  | - name: raw | ||||||
|  |   chart: ../../charts/raw-0.0.1 | ||||||
|  |   values: | ||||||
|  |   - templates: | ||||||
|  |     - | | ||||||
|  |       envName: {{ .Values.envName }} | ||||||
|  | @ -0,0 +1,6 @@ | ||||||
|  | Building dependency release=raw, chart=../../charts/raw-0.0.1 | ||||||
|  | Templating release=raw, chart=../../charts/raw-0.0.1 | ||||||
|  | --- | ||||||
|  | # Source: raw/templates/resources.yaml | ||||||
|  | envName: test | ||||||
|  | 
 | ||||||
|  | @ -0,0 +1 @@ | ||||||
|  | envName: {{ .Environment.Name }} | ||||||
		Loading…
	
		Reference in New Issue