From f7976d9c1add12a88b85058ea3b779cae7632191 Mon Sep 17 00:00:00 2001 From: yxxhero Date: Thu, 15 May 2025 21:31:46 +0800 Subject: [PATCH] fix more issues Signed-off-by: yxxhero --- docs/index.md | 2 +- go.mod | 4 +- pkg/app/app_template_test.go | 18 +++---- pkg/app/app_test.go | 24 +++++----- pkg/envvar/const.go | 2 +- pkg/runtime/runtime.go | 22 ++++----- pkg/state/release_test.go | 6 +-- pkg/tmpl/context_funcs_test.go | 18 +++---- pkg/yaml/yaml.go | 48 ++++++------------- pkg/yaml/yaml_test.go | 18 +++---- test/e2e/template/helmfile/snapshot_test.go | 12 ++--- ...tput.yaml => gopkg.in-yaml.v2-output.yaml} | 0 12 files changed, 77 insertions(+), 97 deletions(-) rename test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/{goccy-go-yaml-output.yaml => gopkg.in-yaml.v2-output.yaml} (100%) diff --git a/docs/index.md b/docs/index.md index e230e86c..678b12b3 100644 --- a/docs/index.md +++ b/docs/index.md @@ -570,7 +570,7 @@ 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.v3*. It's `false` by default in Helmfile until *goccy/go-yaml* is stable enough. +* `HELMFILE_GO_YAML_V3` - use *gopkg.in/yaml.v3* instead of *gopkg.in/yaml.v2*. It's `false` by default in Helmfile v0.x, and `true` in 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 diff --git a/go.mod b/go.mod index 68d6bfaa..8926e106 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,6 @@ require ( github.com/Masterminds/sprig/v3 v3.3.0 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc github.com/go-test/deep v1.1.1 - github.com/goccy/go-yaml v1.17.1 github.com/golang/mock v1.6.0 github.com/google/go-cmp v0.7.0 github.com/gosuri/uitable v0.0.4 @@ -28,6 +27,7 @@ require ( go.uber.org/zap v1.27.0 golang.org/x/sync v0.14.0 golang.org/x/term v0.32.0 + gopkg.in/yaml.v2 v2.4.0 gopkg.in/yaml.v3 v3.0.1 helm.sh/helm/v3 v3.17.3 k8s.io/apimachinery v0.33.0 @@ -217,6 +217,7 @@ require ( github.com/go-openapi/swag v0.23.1 // indirect github.com/go-openapi/validate v0.24.0 // indirect github.com/gobwas/glob v0.2.3 // indirect + github.com/goccy/go-yaml v1.17.1 // indirect github.com/godbus/dbus/v5 v5.1.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang-jwt/jwt/v5 v5.2.2 // indirect @@ -307,7 +308,6 @@ 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 k8s.io/api v0.33.0 // indirect k8s.io/apiextensions-apiserver v0.32.2 // indirect k8s.io/cli-runtime v0.32.2 // indirect diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index 7c4419b1..cf17fec0 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -338,18 +338,18 @@ releases: func TestTemplate_StrictParsing(t *testing.T) { type testcase struct { - goccyGoYaml bool - ns string - error string + GoYamlV3 bool + ns string + error string } check := func(t *testing.T, tc testcase) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = tc.goccyGoYaml + v := runtime.GoYamlV3 + runtime.GoYamlV3 = tc.GoYamlV3 t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) var helm = &exectest.Helm{ @@ -411,9 +411,9 @@ releases: }) } - t.Run("fail due to unknown field with goccy/go-yaml", func(t *testing.T) { + t.Run("fail due to unknown field with gopkg.in/yaml.v3", func(t *testing.T) { check(t, testcase{ - goccyGoYaml: true, + GoYamlV3: 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 @@ -425,7 +425,7 @@ releases: t.Run("fail due to unknown field with gopkg.in/yaml.v3", func(t *testing.T) { check(t, testcase{ - goccyGoYaml: false, + GoYamlV3: 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`, }) diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 2cb36f51..9494c2ad 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -4020,13 +4020,13 @@ 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, GoYamlV3 bool) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml + v := runtime.GoYamlV3 + runtime.GoYamlV3 = GoYamlV3 t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) files := map[string]string{ @@ -4088,13 +4088,13 @@ releases: } } -func testSetValuesTemplate(t *testing.T, goccyGoYaml bool) { +func testSetValuesTemplate(t *testing.T, GoYamlV3 bool) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml + v := runtime.GoYamlV3 + runtime.GoYamlV3 = GoYamlV3 t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) files := map[string]string{ @@ -4161,21 +4161,21 @@ releases: } func TestSetValuesTemplate(t *testing.T) { - t.Run("with goccy/go-yaml", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v3", func(t *testing.T) { testSetValuesTemplate(t, true) }) - t.Run("with gopkg.in/yaml.v3", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { testSetValuesTemplate(t, false) }) } func TestSetStringValuesTemplate(t *testing.T) { - t.Run("with goccy/go-yaml", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v3", func(t *testing.T) { testSetStringValuesTemplate(t, true) }) - t.Run("with gopkg.in/yaml.v3", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { testSetStringValuesTemplate(t, false) }) } diff --git a/pkg/envvar/const.go b/pkg/envvar/const.go index 0235af17..3b5b5cc6 100644 --- a/pkg/envvar/const.go +++ b/pkg/envvar/const.go @@ -12,7 +12,7 @@ const ( FilePath = "HELMFILE_FILE_PATH" TempDir = "HELMFILE_TEMPDIR" UpgradeNoticeDisabled = "HELMFILE_UPGRADE_NOTICE_DISABLED" - GoccyGoYaml = "HELMFILE_GOCCY_GOYAML" + GoYamlV3 = "HELMFILE_GO_YAML_V3" CacheHome = "HELMFILE_CACHE_HOME" Interactive = "HELMFILE_INTERACTIVE" ) diff --git a/pkg/runtime/runtime.go b/pkg/runtime/runtime.go index 83bd13c2..214e046c 100644 --- a/pkg/runtime/runtime.go +++ b/pkg/runtime/runtime.go @@ -8,16 +8,16 @@ import ( ) var ( - // GoccyGoYaml is set to true in order to let Helmfile use - // goccy/go-yaml instead of gopkg.in/yaml.v3. - // It's false by default in Helmfile until the GoccyGoYaml is ready to be used - GoccyGoYaml bool + // GoYamlV3 is set to true in order to let Helmfile use + // gopkg.in/yaml.v3 instead of gopkg.in/yaml.v2. + // It's false by default in Helmfile v0.x and true in Helmfile v1.x. + GoYamlV3 bool ) func Info() string { - yamlLib := "gopkg.in/yaml.v3" - if GoccyGoYaml { - yamlLib = "goccy/go-yaml" + yamlLib := "gopkg.in/yaml.v2" + if GoYamlV3 { + yamlLib = "gopkg.in/yaml.v3" } return fmt.Sprintf("YAML library = %v", yamlLib) @@ -25,12 +25,12 @@ func Info() string { func init() { // You can switch the YAML library at runtime via an envvar: - switch os.Getenv(envvar.GoccyGoYaml) { + switch os.Getenv(envvar.GoYamlV3) { case "true": - GoccyGoYaml = true + GoYamlV3 = true case "false": - GoccyGoYaml = false + GoYamlV3 = false default: - GoccyGoYaml = false + GoYamlV3 = true } } diff --git a/pkg/state/release_test.go b/pkg/state/release_test.go index 721b8351..b1d8011f 100644 --- a/pkg/state/release_test.go +++ b/pkg/state/release_test.go @@ -22,10 +22,10 @@ func TestExecuteTemplateExpressions(t *testing.T) { }, }) - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = true + v := runtime.GoYamlV3 + runtime.GoYamlV3 = true t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) rs := ReleaseSpec{ diff --git a/pkg/tmpl/context_funcs_test.go b/pkg/tmpl/context_funcs_test.go index 377aa916..598e33ad 100644 --- a/pkg/tmpl/context_funcs_test.go +++ b/pkg/tmpl/context_funcs_test.go @@ -160,9 +160,9 @@ func TestReadFile_PassAbsPath(t *testing.T) { } func TestToYaml_NestedMapInterfaceKey(t *testing.T) { - v := runtime.GoccyGoYaml + v := runtime.GoYamlV3 t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) // nolint: unconvert @@ -172,13 +172,13 @@ func TestToYaml_NestedMapInterfaceKey(t *testing.T) { }, }) - runtime.GoccyGoYaml = true + runtime.GoYamlV3 = true actual, err := ToYaml(vals) require.Equal(t, "foo:\n bar: BAR\n", actual) require.NoError(t, err, "expected nil, but got: %v, when type: map[interface {}]interface {}", err) - runtime.GoccyGoYaml = false + runtime.GoYamlV3 = false actual, err = ToYaml(vals) require.Equal(t, "foo:\n bar: BAR\n", actual) @@ -335,13 +335,13 @@ func testFromYamlNull(t *testing.T) { require.Equal(t, nil, actual) } -func testFromYaml(t *testing.T, goccyGoYaml bool) { +func testFromYaml(t *testing.T, GoYamlV3 bool) { t.Helper() - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml + v := runtime.GoYamlV3 + runtime.GoYamlV3 = GoYamlV3 t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) t.Run("test unmarshalling object", testFromYamlObject) @@ -358,7 +358,7 @@ func testFromYaml(t *testing.T, goccyGoYaml bool) { } func TestFromYaml(t *testing.T) { - t.Run("with goccy/go-yaml", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { testFromYaml(t, true) }) diff --git a/pkg/yaml/yaml.go b/pkg/yaml/yaml.go index 72ed4bb2..ffde3622 100644 --- a/pkg/yaml/yaml.go +++ b/pkg/yaml/yaml.go @@ -4,7 +4,7 @@ import ( "bytes" "io" - "github.com/goccy/go-yaml" + v2 "gopkg.in/yaml.v2" v3 "gopkg.in/yaml.v3" "github.com/helmfile/helmfile/pkg/runtime" @@ -17,22 +17,12 @@ 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 { - yamlEncoderOpts := []yaml.EncodeOption{ - yaml.Indent(2), - yaml.UseSingleQuote(true), - yaml.UseLiteralStyleIfMultiline(true), - } - yamlEncoder := yaml.NewEncoder( - w, - yamlEncoderOpts..., - ) - return yamlEncoder + if runtime.GoYamlV3 { + v3Encoder := v3.NewEncoder(w) + v3Encoder.SetIndent(2) + return v3Encoder } - - v3Encoder := v3.NewEncoder(w) - v3Encoder.SetIndent(2) - return v3Encoder + return v2.NewEncoder(w) } func Marshal(v any) ([]byte, error) { @@ -50,26 +40,16 @@ func Marshal(v any) ([]byte, 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..., - ) - + if runtime.GoYamlV3 { + decoder := v3.NewDecoder(bytes.NewReader(data)) + decoder.KnownFields(strict) return func(v any) error { return decoder.Decode(v) } } - decoder := v3.NewDecoder(bytes.NewReader(data)) - decoder.KnownFields(strict) + decoder := v2.NewDecoder(bytes.NewReader(data)) + decoder.SetStrict(strict) return func(v any) error { return decoder.Decode(v) @@ -77,9 +57,9 @@ func NewDecoder(data []byte, strict bool) func(any) error { } func Unmarshal(data []byte, v any) error { - if runtime.GoccyGoYaml { - return yaml.Unmarshal(data, v) + if runtime.GoYamlV3 { + return v3.Unmarshal(data, v) } - return v3.Unmarshal(data, v) + return v2.Unmarshal(data, v) } diff --git a/pkg/yaml/yaml_test.go b/pkg/yaml/yaml_test.go index 05251310..a01cecd9 100644 --- a/pkg/yaml/yaml_test.go +++ b/pkg/yaml/yaml_test.go @@ -8,20 +8,20 @@ import ( "github.com/helmfile/helmfile/pkg/runtime" ) -func testYamlMarshal(t *testing.T, goccyGoYaml bool) { +func testYamlMarshal(t *testing.T, GoYamlV3 bool) { t.Helper() var yamlLibraryName string - if goccyGoYaml { - yamlLibraryName = "goccy/go-yaml" - } else { + if GoYamlV3 { yamlLibraryName = "gopkg.in/yaml.v3" + } else { + yamlLibraryName = "gopkg.in/yaml.v2" } - v := runtime.GoccyGoYaml - runtime.GoccyGoYaml = goccyGoYaml + v := runtime.GoYamlV3 + runtime.GoYamlV3 = GoYamlV3 t.Cleanup(func() { - runtime.GoccyGoYaml = v + runtime.GoYamlV3 = v }) tests := []struct { @@ -49,7 +49,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", "gopkg.in/yaml.v3": "name: John\ninfo:\n - age: 20\n address: New York\n annotation: \"on\"\n", }, }, @@ -63,7 +63,7 @@ func testYamlMarshal(t *testing.T, goccyGoYaml bool) { } func TestYamlMarshal(t *testing.T) { - t.Run("with goccy/go-yaml", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { testYamlMarshal(t, true) }) diff --git a/test/e2e/template/helmfile/snapshot_test.go b/test/e2e/template/helmfile/snapshot_test.go index dc6fbf90..078adafa 100644 --- a/test/e2e/template/helmfile/snapshot_test.go +++ b/test/e2e/template/helmfile/snapshot_test.go @@ -58,7 +58,7 @@ func (f fakeInit) Force() bool { } func TestHelmfileTemplateWithBuildCommand(t *testing.T) { - t.Run("with goccy/go-yaml", func(t *testing.T) { + t.Run("with gopkg.in/yaml.v2", func(t *testing.T) { testHelmfileTemplateWithBuildCommand(t, true) }) @@ -67,8 +67,8 @@ func TestHelmfileTemplateWithBuildCommand(t *testing.T) { }) } -func testHelmfileTemplateWithBuildCommand(t *testing.T, goccyGoYaml bool) { - t.Setenv(envvar.GoccyGoYaml, strconv.FormatBool(goccyGoYaml)) +func testHelmfileTemplateWithBuildCommand(t *testing.T, GoYamlV3 bool) { + t.Setenv(envvar.GoYamlV3, strconv.FormatBool(GoYamlV3)) localChartPortSets := make(map[int]struct{}) @@ -225,10 +225,10 @@ func testHelmfileTemplateWithBuildCommand(t *testing.T, goccyGoYaml bool) { inputFile := filepath.Join(testdataDir, name, "input.yaml.gotmpl") outputFile := "" - if goccyGoYaml { - outputFile = filepath.Join(testdataDir, name, "goccy-go-yaml-output.yaml") - } else { + if GoYamlV3 { outputFile = filepath.Join(testdataDir, name, "gopkg.in-yaml.v3-output.yaml") + } else { + outputFile = filepath.Join(testdataDir, name, "gopkg.in-yaml.v2-output.yaml") } ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second) diff --git a/test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/goccy-go-yaml-output.yaml b/test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/gopkg.in-yaml.v2-output.yaml similarity index 100% rename from test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/goccy-go-yaml-output.yaml rename to test/e2e/template/helmfile/testdata/snapshot/issue_2098_release_template_needs/gopkg.in-yaml.v2-output.yaml