From 2f0fd7a57ce59cc65c19045a5d8c1186cf389559 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Thu, 17 Apr 2025 07:01:06 +0800 Subject: [PATCH] refactor(yaml): switch to goccy/go-yaml library Signed-off-by: yxxhero --- docs/index.md | 5 +- go.mod | 2 +- pkg/app/app_template_test.go | 21 +------ pkg/app/app_test.go | 29 ++------- pkg/envvar/const.go | 1 - pkg/runtime/runtime.go | 20 +----- pkg/tmpl/context_funcs_test.go | 22 +------ pkg/yaml/yaml.go | 70 +++++++-------------- pkg/yaml/yaml_test.go | 26 ++------ test/e2e/template/helmfile/snapshot_test.go | 11 +--- 10 files changed, 41 insertions(+), 166 deletions(-) diff --git a/docs/index.md b/docs/index.md index 749a87f8..79a8b42b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -569,7 +569,6 @@ Helmfile uses some OS environment variables to override default behaviour: * `HELMFILE_ENVIRONMENT` - specify [Helmfile environment](https://helmfile.readthedocs.io/en/latest/#environment), it has lower priority than CLI argument `--environment` * `HELMFILE_TEMPDIR` - specify directory to store temporary files * `HELMFILE_UPGRADE_NOTICE_DISABLED` - expecting any non-empty value to skip the check for the latest version of Helmfile in [helmfile version](https://helmfile.readthedocs.io/en/latest/#version) -* `HELMFILE_GOCCY_GOYAML` - use *goccy/go-yaml* instead of *gopkg.in/yaml.v2*. It's `false` by default in Helmfile v0.x and `true` by default for Helmfile v1.x. * `HELMFILE_CACHE_HOME` - specify directory to store cached files for remote operations * `HELMFILE_FILE_PATH` - specify the path to the helmfile.yaml file * `HELMFILE_INTERACTIVE` - enable interactive mode, expecting `true` lower case. The same as `--interactive` CLI flag @@ -578,8 +577,8 @@ Helmfile uses some OS environment variables to override default behaviour: ``` Declaratively deploy your Kubernetes manifests, Kustomize configs, and Charts as Helm releases in one shot -V1 mode = false -YAML library = gopkg.in/yaml.v2 +V1 mode = true +YAML library = goccy/go-yaml Usage: helmfile [command] diff --git a/go.mod b/go.mod index efc70bd2..82e0c096 100644 --- a/go.mod +++ b/go.mod @@ -28,7 +28,6 @@ require ( go.uber.org/zap v1.27.0 golang.org/x/sync v0.13.0 golang.org/x/term v0.31.0 - gopkg.in/yaml.v2 v2.4.0 helm.sh/helm/v3 v3.17.3 k8s.io/apimachinery v0.32.3 ) @@ -309,6 +308,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/gookit/color.v1 v1.1.6 // indirect gopkg.in/inf.v0 v0.9.1 // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect k8s.io/api v0.32.3 // indirect k8s.io/apiextensions-apiserver v0.32.2 // indirect diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index e42122e1..bf6accaf 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -16,7 +16,6 @@ import ( "github.com/helmfile/helmfile/pkg/exectest" ffs "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" - "github.com/helmfile/helmfile/pkg/runtime" ) func TestTemplate(t *testing.T) { @@ -338,20 +337,13 @@ releases: func TestTemplate_StrictParsing(t *testing.T) { type testcase struct { - goccyGoYaml bool - ns string - error string + ns string + error string } check := func(t *testing.T, tc testcase) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = tc.goccyGoYaml - t.Cleanup(func() { - runtime.GoccyGoYaml = v - }) - var helm = &exectest.Helm{ FailOnUnexpectedList: true, FailOnUnexpectedDiff: true, @@ -413,7 +405,6 @@ releases: t.Run("fail due to unknown field with goccy/go-yaml", func(t *testing.T) { check(t, testcase{ - goccyGoYaml: true, error: `in ./helmfile.yaml: failed to read helmfile.yaml: reading document at index 1. Started seeing this since Helmfile v1? Add the .gotmpl file extension: [4:3] unknown field "foobar" 2 | releases: 3 | - name: app1 @@ -422,14 +413,6 @@ releases: 5 | chart: incubator/raw`, }) }) - - t.Run("fail due to unknown field with gopkg.in/yaml.v2", func(t *testing.T) { - check(t, testcase{ - goccyGoYaml: false, - error: `in ./helmfile.yaml: failed to read helmfile.yaml: reading document at index 1. Started seeing this since Helmfile v1? Add the .gotmpl file extension: yaml: unmarshal errors: - line 4: field foobar not found in type state.ReleaseSpec`, - }) - }) } func TestTemplate_CyclicInheritance(t *testing.T) { diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 67e6f8a2..376b3905 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -26,7 +26,6 @@ import ( ffs "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/remote" - "github.com/helmfile/helmfile/pkg/runtime" "github.com/helmfile/helmfile/pkg/state" "github.com/helmfile/helmfile/pkg/testhelper" "github.com/helmfile/helmfile/pkg/testutil" @@ -4020,15 +4019,9 @@ myrelease4 testNamespace true true chart:mychart1,id:myrelease1,name:myr assert.Equal(t, expected, out) } -func testSetStringValuesTemplate(t *testing.T, goccyGoYaml bool) { +func testSetStringValuesTemplate(t *testing.T) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml - t.Cleanup(func() { - runtime.GoccyGoYaml = v - }) - files := map[string]string{ "/path/to/helmfile.yaml.gotmpl": ` releases: @@ -4088,15 +4081,9 @@ releases: } } -func testSetValuesTemplate(t *testing.T, goccyGoYaml bool) { +func testSetValuesTemplate(t *testing.T) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml - t.Cleanup(func() { - runtime.GoccyGoYaml = v - }) - files := map[string]string{ "/path/to/helmfile.yaml.gotmpl": ` releases: @@ -4162,21 +4149,13 @@ releases: func TestSetValuesTemplate(t *testing.T) { t.Run("with goccy/go-yaml", func(t *testing.T) { - testSetValuesTemplate(t, true) - }) - - t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { - testSetValuesTemplate(t, false) + testSetValuesTemplate(t) }) } func TestSetStringValuesTemplate(t *testing.T) { t.Run("with goccy/go-yaml", func(t *testing.T) { - testSetStringValuesTemplate(t, true) - }) - - t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { - testSetStringValuesTemplate(t, false) + testSetStringValuesTemplate(t) }) } diff --git a/pkg/envvar/const.go b/pkg/envvar/const.go index 0235af17..c9bd9577 100644 --- a/pkg/envvar/const.go +++ b/pkg/envvar/const.go @@ -12,7 +12,6 @@ const ( FilePath = "HELMFILE_FILE_PATH" TempDir = "HELMFILE_TEMPDIR" UpgradeNoticeDisabled = "HELMFILE_UPGRADE_NOTICE_DISABLED" - GoccyGoYaml = "HELMFILE_GOCCY_GOYAML" CacheHome = "HELMFILE_CACHE_HOME" Interactive = "HELMFILE_INTERACTIVE" ) diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 7c1e1d14..c2795ef2 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -2,35 +2,19 @@ package runtime import ( "fmt" - "os" - - "github.com/helmfile/helmfile/pkg/envvar" ) var ( // GoccyGoYaml is set to true in order to let Helmfile use // goccy/go-yaml instead of gopkg.in/yaml.v2. - // It's false by default in Helmfile v0.x and true by default for Helmfile v1.x. GoccyGoYaml bool ) func Info() string { - yamlLib := "gopkg.in/yaml.v2" - if GoccyGoYaml { - yamlLib = "goccy/go-yaml" - } - + yamlLib := "goccy/go-yaml" return fmt.Sprintf("YAML library = %v", yamlLib) } func init() { - // You can switch the YAML library at runtime via an envvar: - switch os.Getenv(envvar.GoccyGoYaml) { - case "true": - GoccyGoYaml = true - case "false": - GoccyGoYaml = false - default: - GoccyGoYaml = true - } + GoccyGoYaml = true } diff --git a/pkg/tmpl/context_funcs_test.go b/pkg/tmpl/context_funcs_test.go index d1141d8e..30581ae9 100644 --- a/pkg/tmpl/context_funcs_test.go +++ b/pkg/tmpl/context_funcs_test.go @@ -200,15 +200,9 @@ func TestToYaml(t *testing.T) { require.Equal(t, expected, actual) } -func testFromYaml(t *testing.T, goccyGoYaml bool, expected Values) { +func testFromYaml(t *testing.T, expected Values) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml - t.Cleanup(func() { - runtime.GoccyGoYaml = v - }) - raw := `foo: bar: BAR ` @@ -221,20 +215,6 @@ func TestFromYaml(t *testing.T) { t.Run("with goccy/go-yaml", func(t *testing.T) { testFromYaml( t, - true, - // nolint: unconvert - Values(map[string]any{ - "foo": map[string]any{ - "bar": "BAR", - }, - }), - ) - }) - - t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { - testFromYaml( - t, - false, // nolint: unconvert Values(map[string]any{ "foo": map[string]any{ diff --git a/pkg/yaml/yaml.go b/pkg/yaml/yaml.go index 06054261..8cc05573 100644 --- a/pkg/yaml/yaml.go +++ b/pkg/yaml/yaml.go @@ -5,9 +5,6 @@ import ( "io" "github.com/goccy/go-yaml" - v2 "gopkg.in/yaml.v2" - - "github.com/helmfile/helmfile/pkg/runtime" ) type Encoder interface { @@ -17,19 +14,11 @@ type Encoder interface { // NewEncoder creates and returns a function that is used to encode a Go object to a YAML document func NewEncoder(w io.Writer) Encoder { - if runtime.GoccyGoYaml { - return yaml.NewEncoder(w) - } - - return v2.NewEncoder(w) + return yaml.NewEncoder(w) } func Unmarshal(data []byte, v any) error { - if runtime.GoccyGoYaml { - return yaml.Unmarshal(data, v) - } - - return v2.Unmarshal(data, v) + return yaml.Unmarshal(data, v) } // NewDecoder creates and returns a function that is used to decode a YAML document @@ -37,26 +26,17 @@ func Unmarshal(data []byte, v any) error { // When strict is true, this function ensures that every field found in the YAML document // to have the corresponding field in the decoded Go struct. func NewDecoder(data []byte, strict bool) func(any) error { - if runtime.GoccyGoYaml { - var opts []yaml.DecodeOption - if strict { - opts = append(opts, yaml.DisallowUnknownField()) - } - // allow duplicate keys - opts = append(opts, yaml.AllowDuplicateMapKey()) - - decoder := yaml.NewDecoder( - bytes.NewReader(data), - opts..., - ) - - return func(v any) error { - return decoder.Decode(v) - } + var opts []yaml.DecodeOption + if strict { + opts = append(opts, yaml.DisallowUnknownField()) } + // allow duplicate keys + opts = append(opts, yaml.AllowDuplicateMapKey()) - decoder := v2.NewDecoder(bytes.NewReader(data)) - decoder.SetStrict(strict) + decoder := yaml.NewDecoder( + bytes.NewReader(data), + opts..., + ) return func(v any) error { return decoder.Decode(v) @@ -64,20 +44,16 @@ func NewDecoder(data []byte, strict bool) func(any) error { } func Marshal(v any) ([]byte, error) { - if runtime.GoccyGoYaml { - var b bytes.Buffer - yamlEncoder := yaml.NewEncoder( - &b, - yaml.Indent(2), - yaml.UseSingleQuote(true), - yaml.UseLiteralStyleIfMultiline(true), - ) - err := yamlEncoder.Encode(v) - defer func() { - _ = yamlEncoder.Close() - }() - return b.Bytes(), err - } - - return v2.Marshal(v) + var b bytes.Buffer + yamlEncoder := yaml.NewEncoder( + &b, + yaml.Indent(2), + yaml.UseSingleQuote(true), + yaml.UseLiteralStyleIfMultiline(true), + ) + err := yamlEncoder.Encode(v) + defer func() { + _ = yamlEncoder.Close() + }() + return b.Bytes(), err } diff --git a/pkg/yaml/yaml_test.go b/pkg/yaml/yaml_test.go index 35e2a3ba..5c71ec8b 100644 --- a/pkg/yaml/yaml_test.go +++ b/pkg/yaml/yaml_test.go @@ -4,25 +4,12 @@ import ( "testing" "github.com/stretchr/testify/require" - - "github.com/helmfile/helmfile/pkg/runtime" ) -func testYamlMarshal(t *testing.T, goccyGoYaml bool) { +func testYamlMarshal(t *testing.T) { t.Helper() - var yamlLibraryName string - if goccyGoYaml { - yamlLibraryName = "goccy/go-yaml" - } else { - yamlLibraryName = "gopkg.in/yaml.v2" - } - - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml - t.Cleanup(func() { - runtime.GoccyGoYaml = v - }) + yamlLibraryName := "goccy/go-yaml" tests := []struct { Name string `yaml:"name"` @@ -49,8 +36,7 @@ func testYamlMarshal(t *testing.T, goccyGoYaml bool) { Annotation: "on", }}, expected: map[string]string{ - "goccy/go-yaml": "name: John\ninfo:\n- age: 20\n address: New York\n annotation: 'on'\n", - "gopkg.in/yaml.v2": "name: John\ninfo:\n- age: 20\n address: New York\n annotation: \"on\"\n", + "goccy/go-yaml": "name: John\ninfo:\n- age: 20\n address: New York\n annotation: 'on'\n", }, }, } @@ -64,10 +50,6 @@ func testYamlMarshal(t *testing.T, goccyGoYaml bool) { func TestYamlMarshal(t *testing.T) { t.Run("with goccy/go-yaml", func(t *testing.T) { - testYamlMarshal(t, true) - }) - - t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { - testYamlMarshal(t, false) + testYamlMarshal(t) }) } diff --git a/test/e2e/template/helmfile/snapshot_test.go b/test/e2e/template/helmfile/snapshot_test.go index 68f62be5..6f3a530d 100644 --- a/test/e2e/template/helmfile/snapshot_test.go +++ b/test/e2e/template/helmfile/snapshot_test.go @@ -9,7 +9,6 @@ import ( "path/filepath" "regexp" goruntime "runtime" - "strconv" "strings" "testing" "time" @@ -59,17 +58,11 @@ func (f fakeInit) Force() bool { func TestHelmfileTemplateWithBuildCommand(t *testing.T) { t.Run("with goccy/go-yaml", func(t *testing.T) { - testHelmfileTemplateWithBuildCommand(t, true) - }) - - t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { - testHelmfileTemplateWithBuildCommand(t, false) + testHelmfileTemplateWithBuildCommand(t) }) } -func testHelmfileTemplateWithBuildCommand(t *testing.T, goccyGoYaml bool) { - t.Setenv(envvar.GoccyGoYaml, strconv.FormatBool(goccyGoYaml)) - +func testHelmfileTemplateWithBuildCommand(t *testing.T) { localChartPortSets := make(map[int]struct{}) logger := helmexec.NewLogger(os.Stderr, "info")