From d8d0bf830a9ccfb2e5065eb83c3b018a4489946f Mon Sep 17 00:00:00 2001 From: yxxhero <11087727+yxxhero@users.noreply.github.com> Date: Wed, 21 Dec 2022 10:49:31 +0800 Subject: [PATCH] Add helmfile state validate policy (#592) --- go.mod | 2 + go.sum | 9 +--- pkg/policy/checker.go | 37 +++++++++++++ pkg/policy/checker_test.go | 54 +++++++++++++++++++ pkg/state/state.go | 25 +++++++++ .../config.yaml | 8 +++ .../input.yaml | 11 ++++ .../output.yaml | 8 +++ .../regression/input/issue.1867.yaml | 2 +- 9 files changed, 148 insertions(+), 8 deletions(-) create mode 100644 pkg/policy/checker.go create mode 100644 pkg/policy/checker_test.go create mode 100644 test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/config.yaml create mode 100644 test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/input.yaml create mode 100644 test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/output.yaml diff --git a/go.mod b/go.mod index c80d2006..7782932c 100644 --- a/go.mod +++ b/go.mod @@ -32,6 +32,8 @@ require ( k8s.io/apimachinery v0.26.0 ) +replace gopkg.in/yaml.v3 => github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22 + require ( cloud.google.com/go v0.102.1 // indirect cloud.google.com/go/compute v1.7.0 // indirect diff --git a/go.sum b/go.sum index 5c6abfbd..f53be255 100644 --- a/go.sum +++ b/go.sum @@ -223,6 +223,8 @@ github.com/cncf/xds/go v0.0.0-20210805033703-aa0b78936158/go.mod h1:eXthEFrGJvWH github.com/cncf/xds/go v0.0.0-20210922020428-25de7278fc84/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20211001041855-01bcc9b48dfe/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= +github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22 h1:uVG5v+c6ndz9seCorYjQmlVlPbh3OMcMWJzAJZWdM/g= +github.com/colega/go-yaml-yaml v0.0.0-20220720070545-aaba007ebc22/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= github.com/containerd/cgroups v1.0.3 h1:ADZftAkglvCiD44c77s5YmMqaP2pzVCFZvBmAlBdAP4= github.com/containerd/containerd v1.6.6 h1:xJNPhbrmz8xAMDNoVjHy9YHtWwEQNS+CDkcIRh7t8Y0= github.com/containerd/containerd v1.6.6/go.mod h1:ZoP1geJldzCVY3Tonoz7b1IXk8rIX0Nltt5QE4OMNk0= @@ -1391,13 +1393,6 @@ gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.3.0/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= -gopkg.in/yaml.v3 v3.0.0-20200121175148-a6ecf24a6d71/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0-20210107172259-749611fa9fcc/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.0/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gotest.tools v2.2.0+incompatible h1:VsBPFP1AI068pPrMxtb/S8Zkgf9xEmTLJjfM+P5UIEo= gotest.tools v2.2.0+incompatible/go.mod h1:DsYFclhRJ6vuDpmuTbkuFWG+y2sxOXAzmJt81HFBacw= gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk= diff --git a/pkg/policy/checker.go b/pkg/policy/checker.go new file mode 100644 index 00000000..98475682 --- /dev/null +++ b/pkg/policy/checker.go @@ -0,0 +1,37 @@ +// Package policy provides a policy checker for the helmfile state. +package policy + +import ( + "errors" +) + +var ( + EnvironmentsAndReleasesWithinSameYamlPartErr = errors.New("environments and releases cannot be defined within the same YAML part. Use --- to extract the environments into a dedicated part") +) + +// checkerFunc is a function that checks the helmState. +type checkerFunc func(map[string]interface{}) (bool, error) + +func forbidEnvironmentsWithReleases(releaseState map[string]interface{}) (bool, error) { + // forbid environments and releases to be defined at the same yaml part + _, hasEnvironments := releaseState["environments"] + _, hasReleases := releaseState["releases"] + if hasEnvironments && hasReleases { + return false, EnvironmentsAndReleasesWithinSameYamlPartErr + } + return false, nil +} + +var checkerFuncs = []checkerFunc{ + forbidEnvironmentsWithReleases, +} + +// Checker is a policy checker for the helmfile state. +func Checker(helmState map[string]interface{}) (bool, error) { + for _, fn := range checkerFuncs { + if isStrict, err := fn(helmState); err != nil { + return isStrict, err + } + } + return false, nil +} diff --git a/pkg/policy/checker_test.go b/pkg/policy/checker_test.go new file mode 100644 index 00000000..6ea35366 --- /dev/null +++ b/pkg/policy/checker_test.go @@ -0,0 +1,54 @@ +package policy + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestForbidEnvironmentsWithReleases(t *testing.T) { + testCases := []struct { + name string + helmState map[string]interface{} + expectedErr bool + isStrict bool + }{ + { + name: "no error when only releases", + helmState: map[string]interface{}{ + "releases": interface{}(nil), + }, + expectedErr: false, + isStrict: false, + }, + { + name: "no error when only environments", + helmState: map[string]interface{}{ + "environments": map[string]interface{}{}, + }, + expectedErr: false, + isStrict: false, + }, + { + name: "error when both releases and environments", + helmState: map[string]interface{}{ + "environments": interface{}(nil), + "releases": interface{}(nil), + }, + expectedErr: true, + isStrict: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + isStrict, err := forbidEnvironmentsWithReleases(tc.helmState) + require.Equal(t, tc.isStrict, isStrict, "expected isStrict=%v, got=%v", tc.isStrict, isStrict) + if tc.expectedErr { + require.ErrorIsf(t, err, EnvironmentsAndReleasesWithinSameYamlPartErr, "expected error=%v, got=%v", EnvironmentsAndReleasesWithinSameYamlPartErr, err) + } else { + require.NoError(t, err, "expected no error but got error: %v", err) + } + }) + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 246b3901..39842a03 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -29,6 +29,7 @@ import ( "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/maputil" + "github.com/helmfile/helmfile/pkg/policy" "github.com/helmfile/helmfile/pkg/remote" "github.com/helmfile/helmfile/pkg/tmpl" ) @@ -83,6 +84,30 @@ type ReleaseSetSpec struct { LockFile string `yaml:"lockFilePath,omitempty"` } +// helmStateAlias is helm state alias +type helmStateAlias HelmState + +func (hs HelmState) MarshalYAML() (interface{}, error) { + return helmStateAlias(hs), nil +} + +func (hs *HelmState) UnmarshalYAML(value *yaml.Node) error { + helmStateInfo := make(map[string]interface{}) + if err := value.DecodeWithOptions(&helmStateInfo, yaml.DecodeOptions{KnownFields: true}); err != nil { + return err + } + + isStrict, err := policy.Checker(helmStateInfo) + if err != nil { + if isStrict { + return err + } + fmt.Fprintf(os.Stderr, "Warning: %v\n", err) + } + + return value.DecodeWithOptions((*helmStateAlias)(hs), yaml.DecodeOptions{KnownFields: true}) +} + // HelmState structure for the helmfile type HelmState struct { basePath string diff --git a/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/config.yaml new file mode 100644 index 00000000..f19f5e89 --- /dev/null +++ b/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/config.yaml @@ -0,0 +1,8 @@ +localChartRepoServer: + enabled: true + port: 18083 +chartifyTempDir: envs_releases_within_same_yaml_part +helmfileArgs: +- --environment +- prod +- template diff --git a/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/input.yaml b/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/input.yaml new file mode 100644 index 00000000..276a4d93 --- /dev/null +++ b/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/input.yaml @@ -0,0 +1,11 @@ +environments: + prod: + staging: + +releases: +- name: raw + chart: ../../charts/raw-0.0.1 + values: + - templates: + - | + chartVersion: {{`{{ .Chart.Version }}`}} diff --git a/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/output.yaml new file mode 100644 index 00000000..be8b8838 --- /dev/null +++ b/test/e2e/template/helmfile/testdata/snapshot/environments_releases_within_same_yaml_part/output.yaml @@ -0,0 +1,8 @@ +Warning: environments and releases cannot be defined within the same YAML part. Use --- to extract the environments into a dedicated part +Warning: environments and releases cannot be defined within the same YAML part. Use --- to extract the environments into a dedicated part +Building dependency release=raw, chart=../../charts/raw-0.0.1 +Templating release=raw, chart=../../charts/raw-0.0.1 +--- +# Source: raw/templates/resources.yaml +chartVersion: 0.0.1 + diff --git a/test/integration/test-cases/regression/input/issue.1867.yaml b/test/integration/test-cases/regression/input/issue.1867.yaml index 3423a7af..7383675d 100644 --- a/test/integration/test-cases/regression/input/issue.1867.yaml +++ b/test/integration/test-cases/regression/input/issue.1867.yaml @@ -5,7 +5,7 @@ repositories: releases: - name: elasticsearch chart: bitnami/elasticsearch - version: 17.5.7 + version: 19.0.1 jsonPatches: - target: version: v1