Relax the forbid-env-with-releases policy for plain helmfile.yaml on v1 (#684)
This commit is contained in:
parent
e930deffbc
commit
8f2d97a1b3
|
|
@ -6,13 +6,13 @@ Note that every breaking change should have an easy alternative way to achieve t
|
||||||
|
|
||||||
## The changes in 1.0
|
## The changes in 1.0
|
||||||
|
|
||||||
1. [Forbid the use of `environments` and `releases` within a single helmfile.yaml part](#forbid-the-use-of-environments-and-releases-within-a-single-helmfileyaml-part)
|
1. [Forbid the use of `environments` and `releases` within a single helmfile.yaml.gotmpl part](#forbid-the-use-of-environments-and-releases-within-a-single-helmfileyamlgotmpl-part)
|
||||||
2. [Force `.gotmpl` file extension for `helmfile.yaml` in case you want helmfile to render it as a go template before yaml parsing.](#force-gotmpl-file-extension-for-helmfileyaml-in-case-you-want-helmfile-to-render-it-as-a-go-template-before-yaml-parsing)
|
2. [Force `.gotmpl` file extension for `helmfile.yaml` in case you want helmfile to render it as a go template before yaml parsing.](#force-gotmpl-file-extension-for-helmfileyaml-in-case-you-want-helmfile-to-render-it-as-a-go-template-before-yaml-parsing)
|
||||||
3. [Remove the `--args` flag from the `helmfile` command](#remove-the---args-flag-from-the-helmfile-command)
|
3. [Remove the `--args` flag from the `helmfile` command](#remove-the---args-flag-from-the-helmfile-command)
|
||||||
4. [Remove `HELMFILE_SKIP_INSECURE_TEMPLATE_FUNCTIONS` in favor of `HELMFILE_DISABLE_INSECURE_FEATURES`](#remove-helmfile_skip_insecure_template_functions-in-favor-of-helmfile_disable_insecure_features)
|
4. [Remove `HELMFILE_SKIP_INSECURE_TEMPLATE_FUNCTIONS` in favor of `HELMFILE_DISABLE_INSECURE_FEATURES`](#remove-helmfile_skip_insecure_template_functions-in-favor-of-helmfile_disable_insecure_features)
|
||||||
5. [The long deprecated `charts.yaml` has been finally removed](#the-long-deprecated-chartsyaml-has-been-finally-removed)
|
5. [The long deprecated `charts.yaml` has been finally removed](#the-long-deprecated-chartsyaml-has-been-finally-removed)
|
||||||
|
|
||||||
### Forbid the use of `environments` and `releases` within a single helmfile.yaml part
|
### Forbid the use of `environments` and `releases` within a single helmfile.yaml.gotmpl part
|
||||||
|
|
||||||
- Helmfile currently relies on a hack called "double rendering" which no one understands correctly (I suppose) to solve the chicken-and-egg problem of rendering the helmfile template(which requires helmfile to parse `environments` as yaml first) and parsing the rendered helmfile as yaml(which requires helmfile to render the template first).
|
- Helmfile currently relies on a hack called "double rendering" which no one understands correctly (I suppose) to solve the chicken-and-egg problem of rendering the helmfile template(which requires helmfile to parse `environments` as yaml first) and parsing the rendered helmfile as yaml(which requires helmfile to render the template first).
|
||||||
- By forcing (or print a big warning) the user to do separate helmfile parts for `environments` and `releases`, it's very unlikely Helmfile needs double-rendering at all.
|
- By forcing (or print a big warning) the user to do separate helmfile parts for `environments` and `releases`, it's very unlikely Helmfile needs double-rendering at all.
|
||||||
|
|
|
||||||
|
|
@ -3,6 +3,7 @@ package policy
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
|
"path/filepath"
|
||||||
|
|
||||||
"github.com/helmfile/helmfile/pkg/runtime"
|
"github.com/helmfile/helmfile/pkg/runtime"
|
||||||
)
|
)
|
||||||
|
|
@ -12,13 +13,13 @@ var (
|
||||||
)
|
)
|
||||||
|
|
||||||
// checkerFunc is a function that checks the helmState.
|
// checkerFunc is a function that checks the helmState.
|
||||||
type checkerFunc func(map[string]interface{}) (bool, error)
|
type checkerFunc func(string, map[string]interface{}) (bool, error)
|
||||||
|
|
||||||
func forbidEnvironmentsWithReleases(releaseState map[string]interface{}) (bool, error) {
|
func forbidEnvironmentsWithReleases(filePath string, releaseState map[string]interface{}) (bool, error) {
|
||||||
// forbid environments and releases to be defined at the same yaml part
|
// forbid environments and releases to be defined at the same yaml part
|
||||||
_, hasEnvironments := releaseState["environments"]
|
_, hasEnvironments := releaseState["environments"]
|
||||||
_, hasReleases := releaseState["releases"]
|
_, hasReleases := releaseState["releases"]
|
||||||
if hasEnvironments && hasReleases {
|
if hasEnvironments && hasReleases && (filepath.Ext(filePath) == ".gotmpl" || !runtime.V1Mode) {
|
||||||
return runtime.V1Mode, EnvironmentsAndReleasesWithinSameYamlPartErr
|
return runtime.V1Mode, EnvironmentsAndReleasesWithinSameYamlPartErr
|
||||||
}
|
}
|
||||||
return false, nil
|
return false, nil
|
||||||
|
|
@ -29,9 +30,9 @@ var checkerFuncs = []checkerFunc{
|
||||||
}
|
}
|
||||||
|
|
||||||
// Checker is a policy checker for the helmfile state.
|
// Checker is a policy checker for the helmfile state.
|
||||||
func Checker(helmState map[string]interface{}) (bool, error) {
|
func Checker(filePath string, helmState map[string]interface{}) (bool, error) {
|
||||||
for _, fn := range checkerFuncs {
|
for _, fn := range checkerFuncs {
|
||||||
if isStrict, err := fn(helmState); err != nil {
|
if isStrict, err := fn(filePath, helmState); err != nil {
|
||||||
return isStrict, err
|
return isStrict, err
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -4,17 +4,23 @@ import (
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/helmfile/helmfile/pkg/runtime"
|
||||||
)
|
)
|
||||||
|
|
||||||
func TestForbidEnvironmentsWithReleases(t *testing.T) {
|
func TestForbidEnvironmentsWithReleases(t *testing.T) {
|
||||||
testCases := []struct {
|
testCases := []struct {
|
||||||
name string
|
name string
|
||||||
|
filePath string
|
||||||
|
v1mode bool
|
||||||
helmState map[string]interface{}
|
helmState map[string]interface{}
|
||||||
expectedErr bool
|
expectedErr bool
|
||||||
isStrict bool
|
isStrict bool
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "no error when only releases",
|
name: "no error when only releases",
|
||||||
|
filePath: "helmfile.yaml",
|
||||||
|
v1mode: false,
|
||||||
helmState: map[string]interface{}{
|
helmState: map[string]interface{}{
|
||||||
"releases": interface{}(nil),
|
"releases": interface{}(nil),
|
||||||
},
|
},
|
||||||
|
|
@ -22,7 +28,9 @@ func TestForbidEnvironmentsWithReleases(t *testing.T) {
|
||||||
isStrict: false,
|
isStrict: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "no error when only environments",
|
name: "no error when only environments",
|
||||||
|
filePath: "helmfile.yaml",
|
||||||
|
v1mode: false,
|
||||||
helmState: map[string]interface{}{
|
helmState: map[string]interface{}{
|
||||||
"environments": map[string]interface{}{},
|
"environments": map[string]interface{}{},
|
||||||
},
|
},
|
||||||
|
|
@ -30,7 +38,9 @@ func TestForbidEnvironmentsWithReleases(t *testing.T) {
|
||||||
isStrict: false,
|
isStrict: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "error when both releases and environments",
|
name: "error when both releases and environments",
|
||||||
|
filePath: "helmfile.yaml",
|
||||||
|
v1mode: false,
|
||||||
helmState: map[string]interface{}{
|
helmState: map[string]interface{}{
|
||||||
"environments": interface{}(nil),
|
"environments": interface{}(nil),
|
||||||
"releases": interface{}(nil),
|
"releases": interface{}(nil),
|
||||||
|
|
@ -38,11 +48,28 @@ func TestForbidEnvironmentsWithReleases(t *testing.T) {
|
||||||
expectedErr: true,
|
expectedErr: true,
|
||||||
isStrict: false,
|
isStrict: false,
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "no error when both releases and environments for plain yaml on v1",
|
||||||
|
filePath: "helmfile.yaml",
|
||||||
|
v1mode: true,
|
||||||
|
helmState: map[string]interface{}{
|
||||||
|
"environments": interface{}(nil),
|
||||||
|
"releases": interface{}(nil),
|
||||||
|
},
|
||||||
|
expectedErr: false,
|
||||||
|
isStrict: false,
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
|
v1mode := runtime.V1Mode
|
||||||
|
t.Cleanup(func() {
|
||||||
|
runtime.V1Mode = v1mode
|
||||||
|
})
|
||||||
|
|
||||||
for _, tc := range testCases {
|
for _, tc := range testCases {
|
||||||
t.Run(tc.name, func(t *testing.T) {
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
isStrict, err := forbidEnvironmentsWithReleases(tc.helmState)
|
runtime.V1Mode = tc.v1mode
|
||||||
|
isStrict, err := forbidEnvironmentsWithReleases(tc.filePath, tc.helmState)
|
||||||
require.Equal(t, tc.isStrict, isStrict, "expected isStrict=%v, got=%v", tc.isStrict, isStrict)
|
require.Equal(t, tc.isStrict, isStrict, "expected isStrict=%v, got=%v", tc.isStrict, isStrict)
|
||||||
if tc.expectedErr {
|
if tc.expectedErr {
|
||||||
require.ErrorIsf(t, err, EnvironmentsAndReleasesWithinSameYamlPartErr, "expected error=%v, got=%v", EnvironmentsAndReleasesWithinSameYamlPartErr, err)
|
require.ErrorIsf(t, err, EnvironmentsAndReleasesWithinSameYamlPartErr, "expected error=%v, got=%v", EnvironmentsAndReleasesWithinSameYamlPartErr, err)
|
||||||
|
|
|
||||||
|
|
@ -96,6 +96,8 @@ func (c *StateCreator) Parse(content []byte, baseDir, file string) (*HelmState,
|
||||||
|
|
||||||
var intermediate HelmState
|
var intermediate HelmState
|
||||||
|
|
||||||
|
intermediate.FilePath = file
|
||||||
|
|
||||||
err := decode(&intermediate)
|
err := decode(&intermediate)
|
||||||
if err == io.EOF {
|
if err == io.EOF {
|
||||||
break
|
break
|
||||||
|
|
|
||||||
|
|
@ -104,7 +104,7 @@ func (hs *HelmState) UnmarshalYAML(unmarshal func(interface{}) error) error {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
|
|
||||||
isStrict, err := policy.Checker(helmStateInfo)
|
isStrict, err := policy.Checker(hs.FilePath, helmStateInfo)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if isStrict {
|
if isStrict {
|
||||||
return err
|
return err
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue