Add validation of updateStrategy parameter and unit test
Signed-off-by: Simon Bouchard <sbouchard@rbbn.com>
This commit is contained in:
		
							parent
							
								
									ae87c7a456
								
							
						
					
					
						commit
						c10bb1c4e0
					
				|  | @ -220,6 +220,97 @@ releases: | ||||||
| 	} | 	} | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | func TestUpdateStrategyParamValidation(t *testing.T) { | ||||||
|  | 	cases := []struct { | ||||||
|  | 		files          map[string]string | ||||||
|  | 		updateStrategy string | ||||||
|  | 		isValid        bool | ||||||
|  | 	}{ | ||||||
|  | 		{map[string]string{ | ||||||
|  | 			"/path/to/helmfile.yaml": `releases: | ||||||
|  | - name: zipkin | ||||||
|  |   chart: stable/zipkin | ||||||
|  |   updateStrategy: reinstall | ||||||
|  | `}, | ||||||
|  | 			"reinstall", | ||||||
|  | 			true}, | ||||||
|  | 		{map[string]string{ | ||||||
|  | 			"/path/to/helmfile.yaml": `releases: | ||||||
|  | - name: zipkin | ||||||
|  |   chart: stable/zipkin | ||||||
|  |   updateStrategy: reinstallIfForbidden | ||||||
|  | `}, | ||||||
|  | 			"reinstallIfForbidden", | ||||||
|  | 			true}, | ||||||
|  | 		{map[string]string{ | ||||||
|  | 			"/path/to/helmfile.yaml": `releases: | ||||||
|  | - name: zipkin | ||||||
|  |   chart: stable/zipkin | ||||||
|  |   updateStrategy: | ||||||
|  | `}, | ||||||
|  | 			"", | ||||||
|  | 			true}, | ||||||
|  | 		{map[string]string{ | ||||||
|  | 			"/path/to/helmfile.yaml": `releases: | ||||||
|  | - name: zipkin | ||||||
|  |   chart: stable/zipkin | ||||||
|  |   updateStrategy: foo | ||||||
|  | `}, | ||||||
|  | 			"foo", | ||||||
|  | 			false}, | ||||||
|  | 		{map[string]string{ | ||||||
|  | 			"/path/to/helmfile.yaml": `releases: | ||||||
|  | - name: zipkin | ||||||
|  |   chart: stable/zipkin | ||||||
|  |   updateStrategy: reinstal | ||||||
|  | `}, | ||||||
|  | 			"reinstal", | ||||||
|  | 			false}, | ||||||
|  | 		{map[string]string{ | ||||||
|  | 			"/path/to/helmfile.yaml": `releases: | ||||||
|  | - name: zipkin | ||||||
|  |   chart: stable/zipkin | ||||||
|  |   updateStrategy: reinstall1 | ||||||
|  | `}, | ||||||
|  | 			"reinstall1", | ||||||
|  | 			false}, | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
|  | 	for idx, c := range cases { | ||||||
|  | 		fs := testhelper.NewTestFs(c.files) | ||||||
|  | 		app := &App{ | ||||||
|  | 			OverrideHelmBinary:  DefaultHelmBinary, | ||||||
|  | 			OverrideKubeContext: "default", | ||||||
|  | 			Logger:              newAppTestLogger(), | ||||||
|  | 			Namespace:           "", | ||||||
|  | 			Env:                 "default", | ||||||
|  | 			FileOrDir:           "helmfile.yaml", | ||||||
|  | 		} | ||||||
|  | 
 | ||||||
|  | 		expectNoCallsToHelm(app) | ||||||
|  | 
 | ||||||
|  | 		app = injectFs(app, fs) | ||||||
|  | 
 | ||||||
|  | 		err := app.ForEachState( | ||||||
|  | 			Noop, | ||||||
|  | 			false, | ||||||
|  | 			SetFilter(true), | ||||||
|  | 		) | ||||||
|  | 
 | ||||||
|  | 		if c.isValid && err != nil { | ||||||
|  | 			t.Errorf("[case: %d] Unexpected error for valid case: %v", idx, err) | ||||||
|  | 		} else if !c.isValid { | ||||||
|  | 			var invalidUpdateStrategy state.InvalidUpdateStrategyError | ||||||
|  | 			invalidUpdateStrategy.UpdateStrategy = c.updateStrategy | ||||||
|  | 			if err == nil { | ||||||
|  | 				t.Errorf("[case: %d] Expected error for invalid case", idx) | ||||||
|  | 			} else if !strings.Contains(err.Error(), invalidUpdateStrategy.Error()) { | ||||||
|  | 				t.Errorf("[case: %d] Unexpected error returned for invalid case\ngot: %v\nexpected underlying error: %s", idx, err, invalidUpdateStrategy.Error()) | ||||||
|  | 			} | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  | 
 | ||||||
| func TestVisitDesiredStatesWithReleasesFiltered_Issue1008_MissingNonDefaultEnvInBase(t *testing.T) { | func TestVisitDesiredStatesWithReleasesFiltered_Issue1008_MissingNonDefaultEnvInBase(t *testing.T) { | ||||||
| 	files := map[string]string{ | 	files := map[string]string{ | ||||||
| 		"/path/to/base.yaml": ` | 		"/path/to/base.yaml": ` | ||||||
|  |  | ||||||
|  | @ -5,6 +5,7 @@ import ( | ||||||
| 	"errors" | 	"errors" | ||||||
| 	"fmt" | 	"fmt" | ||||||
| 	"path/filepath" | 	"path/filepath" | ||||||
|  | 	"slices" | ||||||
| 
 | 
 | ||||||
| 	"dario.cat/mergo" | 	"dario.cat/mergo" | ||||||
| 	"github.com/helmfile/vals" | 	"github.com/helmfile/vals" | ||||||
|  | @ -285,6 +286,18 @@ func (ld *desiredStateLoader) load(env, overrodeEnv *environment.Environment, ba | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
|  | 	// Validate updateStrategy value if set in the releases
 | ||||||
|  | 	for i := range finalState.Releases { | ||||||
|  | 		if finalState.Releases[i].UpdateStrategy != "" { | ||||||
|  | 			if !slices.Contains(state.ValidUpdateStrategyValues, finalState.Releases[i].UpdateStrategy) { | ||||||
|  | 				return nil, &state.StateLoadError{ | ||||||
|  | 					Msg:   fmt.Sprintf("failed to read %s", finalState.FilePath), | ||||||
|  | 					Cause: &state.InvalidUpdateStrategyError{UpdateStrategy: finalState.Releases[i].UpdateStrategy}, | ||||||
|  | 				} | ||||||
|  | 			} | ||||||
|  | 		} | ||||||
|  | 	} | ||||||
|  | 
 | ||||||
| 	finalState.OrginReleases = finalState.Releases | 	finalState.OrginReleases = finalState.Releases | ||||||
| 	return finalState, nil | 	return finalState, nil | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -26,6 +26,8 @@ const ( | ||||||
| 	DefaultHCLFileExtension = ".hcl" | 	DefaultHCLFileExtension = ".hcl" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
|  | var ValidUpdateStrategyValues = []string{UpdateStrategyReinstall, UpdateStrategyReinstallIfForbidden} | ||||||
|  | 
 | ||||||
| type StateLoadError struct { | type StateLoadError struct { | ||||||
| 	Msg   string | 	Msg   string | ||||||
| 	Cause error | 	Cause error | ||||||
|  | @ -43,6 +45,14 @@ func (e *UndefinedEnvError) Error() string { | ||||||
| 	return fmt.Sprintf("environment \"%s\" is not defined", e.Env) | 	return fmt.Sprintf("environment \"%s\" is not defined", e.Env) | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | type InvalidUpdateStrategyError struct { | ||||||
|  | 	UpdateStrategy string | ||||||
|  | } | ||||||
|  | 
 | ||||||
|  | func (e *InvalidUpdateStrategyError) Error() string { | ||||||
|  | 	return fmt.Sprintf("updateStrategy %q is invalid, valid values are: %s or not set", e.UpdateStrategy, strings.Join(ValidUpdateStrategyValues, ", ")) | ||||||
|  | } | ||||||
|  | 
 | ||||||
| type StateCreator struct { | type StateCreator struct { | ||||||
| 	logger *zap.SugaredLogger | 	logger *zap.SugaredLogger | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -43,6 +43,10 @@ const ( | ||||||
| 	// This is used by an interim solution to make the urfave/cli command report to the helmfile internal about that the
 | 	// This is used by an interim solution to make the urfave/cli command report to the helmfile internal about that the
 | ||||||
| 	// --timeout flag is missingl
 | 	// --timeout flag is missingl
 | ||||||
| 	EmptyTimeout = -1 | 	EmptyTimeout = -1 | ||||||
|  | 
 | ||||||
|  | 	// Valid enum for updateStrategy values
 | ||||||
|  | 	UpdateStrategyReinstall            = "reinstall" | ||||||
|  | 	UpdateStrategyReinstallIfForbidden = "reinstallIfForbidden" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
| // ReleaseSetSpec is release set spec
 | // ReleaseSetSpec is release set spec
 | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue