From e241be85ba748163268eaeed2a88c8e295f84b28 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Fri, 30 Jun 2023 15:59:06 +0400 Subject: [PATCH] fix: properly handle YAML comment stripping for multi-doc Fixes #7425 The previously used method doesn't handle YAML multi-doc, incorrectly stripping only the first document and throwing away everything else. Signed-off-by: Andrey Smirnov --- cmd/talosctl/cmd/talos/edit.go | 6 +- cmd/talosctl/cmd/talos/patch.go | 6 +- .../yamlstrip/testdata/malformed.in.yaml | 4 + .../yamlstrip/testdata/malformed.out.yaml | 3 + .../talos/yamlstrip/testdata/multidoc.in.yaml | 18 ++++ .../yamlstrip/testdata/multidoc.out.yaml | 14 +++ cmd/talosctl/pkg/talos/yamlstrip/yamlstrip.go | 87 +++++++++++++++++++ .../pkg/talos/yamlstrip/yamlstrip_test.go | 36 ++++++++ 8 files changed, 168 insertions(+), 6 deletions(-) create mode 100644 cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.in.yaml create mode 100644 cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.out.yaml create mode 100644 cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.in.yaml create mode 100644 cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.out.yaml create mode 100644 cmd/talosctl/pkg/talos/yamlstrip/yamlstrip.go create mode 100644 cmd/talosctl/pkg/talos/yamlstrip/yamlstrip_test.go diff --git a/cmd/talosctl/cmd/talos/edit.go b/cmd/talosctl/cmd/talos/edit.go index d0ca80043..f90130182 100644 --- a/cmd/talosctl/cmd/talos/edit.go +++ b/cmd/talosctl/cmd/talos/edit.go @@ -19,11 +19,11 @@ import ( "github.com/spf13/cobra" "google.golang.org/protobuf/types/known/durationpb" "gopkg.in/yaml.v3" - cmdutil "k8s.io/kubectl/pkg/cmd/util" "k8s.io/kubectl/pkg/cmd/util/editor" "k8s.io/kubectl/pkg/cmd/util/editor/crlf" "github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers" + "github.com/siderolabs/talos/cmd/talosctl/pkg/talos/yamlstrip" "github.com/siderolabs/talos/pkg/machinery/api/machine" "github.com/siderolabs/talos/pkg/machinery/client" "github.com/siderolabs/talos/pkg/machinery/constants" @@ -111,7 +111,7 @@ func editFn(c *client.Client) func(context.Context, string, resource.Resource, e edited = stripEditingComment(edited) // If we're retrying the loop because of an error, and no change was made in the file, short-circuit - if lastError != "" && bytes.Equal(cmdutil.StripComments(editedDiff), cmdutil.StripComments(edited)) { + if lastError != "" && bytes.Equal(yamlstrip.Comments(editedDiff), yamlstrip.Comments(edited)) { if _, err = os.Stat(path); !os.IsNotExist(err) { message := addEditingComment(lastError) message += fmt.Sprintf("A copy of your changes has been stored to %q\nEdit canceled, no valid changes were saved.\n", path) @@ -120,7 +120,7 @@ func editFn(c *client.Client) func(context.Context, string, resource.Resource, e } } - if len(bytes.TrimSpace(bytes.TrimSpace(cmdutil.StripComments(edited)))) == 0 { + if len(bytes.TrimSpace(bytes.TrimSpace(yamlstrip.Comments(edited)))) == 0 { fmt.Fprintln(os.Stderr, "Apply was skipped: empty file.") break diff --git a/cmd/talosctl/cmd/talos/patch.go b/cmd/talosctl/cmd/talos/patch.go index d8b5d2658..8ae5104fa 100644 --- a/cmd/talosctl/cmd/talos/patch.go +++ b/cmd/talosctl/cmd/talos/patch.go @@ -15,9 +15,9 @@ import ( "github.com/spf13/cobra" "google.golang.org/protobuf/types/known/durationpb" "gopkg.in/yaml.v3" - cmdutil "k8s.io/kubectl/pkg/cmd/util" "github.com/siderolabs/talos/cmd/talosctl/pkg/talos/helpers" + "github.com/siderolabs/talos/cmd/talosctl/pkg/talos/yamlstrip" "github.com/siderolabs/talos/pkg/machinery/api/machine" "github.com/siderolabs/talos/pkg/machinery/client" "github.com/siderolabs/talos/pkg/machinery/config/configpatcher" @@ -67,8 +67,8 @@ func patchFn(c *client.Client, patches []configpatcher.Patch) func(context.Conte }) if bytes.Equal( - bytes.TrimSpace(cmdutil.StripComments(patched)), - bytes.TrimSpace(cmdutil.StripComments(body)), + bytes.TrimSpace(yamlstrip.Comments(patched)), + bytes.TrimSpace(yamlstrip.Comments(body)), ) { fmt.Fprintln(os.Stderr, "Apply was skipped: no changes detected.") diff --git a/cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.in.yaml b/cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.in.yaml new file mode 100644 index 000000000..ad913906a --- /dev/null +++ b/cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.in.yaml @@ -0,0 +1,4 @@ +data: + # This is a comment + some: + other: a: b: c diff --git a/cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.out.yaml b/cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.out.yaml new file mode 100644 index 000000000..e9f4fdd65 --- /dev/null +++ b/cmd/talosctl/pkg/talos/yamlstrip/testdata/malformed.out.yaml @@ -0,0 +1,3 @@ +data: + some: + other: a: b: c diff --git a/cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.in.yaml b/cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.in.yaml new file mode 100644 index 000000000..c71b9ab56 --- /dev/null +++ b/cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.in.yaml @@ -0,0 +1,18 @@ +# siderolink config +apiVersion: v1alpha1 +kind: SideroLinkConfig # kind of the document +# apiUrl is the URL of the SideroLink API endpoint +apiUrl: grpc://172.20.0.1:4000/?jointoken=foo +--- +apiVersion: v1alpha1 +kind: KmsgLogConfig +name: apiSink # named document +url: tcp://[fdae:41e4:649b:9303::1]:4001/ +options: # options are optional + # more options + foo: bar # this option +--- +apiVersion: v1alpha1 +kind: EventSinkConfig +endpoint: "[fdae:41e4:649b:9303::1]:8080" +# end of document diff --git a/cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.out.yaml b/cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.out.yaml new file mode 100644 index 000000000..77a5a4d81 --- /dev/null +++ b/cmd/talosctl/pkg/talos/yamlstrip/testdata/multidoc.out.yaml @@ -0,0 +1,14 @@ +apiVersion: v1alpha1 +kind: SideroLinkConfig +apiUrl: grpc://172.20.0.1:4000/?jointoken=foo +--- +apiVersion: v1alpha1 +kind: KmsgLogConfig +name: apiSink +url: tcp://[fdae:41e4:649b:9303::1]:4001/ +options: + foo: bar +--- +apiVersion: v1alpha1 +kind: EventSinkConfig +endpoint: "[fdae:41e4:649b:9303::1]:8080" diff --git a/cmd/talosctl/pkg/talos/yamlstrip/yamlstrip.go b/cmd/talosctl/pkg/talos/yamlstrip/yamlstrip.go new file mode 100644 index 000000000..a4902c2f3 --- /dev/null +++ b/cmd/talosctl/pkg/talos/yamlstrip/yamlstrip.go @@ -0,0 +1,87 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +// Package yamlstrip provides YAML file manipulation. +package yamlstrip + +import ( + "bytes" + "errors" + "io" + + "gopkg.in/yaml.v3" +) + +// Comments strips comments from a YAML file. +// +// If the YAML file is parseable, it will be accurately stripped. Otherwise, it +// will be stripped in a best-effort manner. +func Comments(b []byte) []byte { + stripped, err := stripViaDecoding(b) + if err != nil { + stripped = stripManual(b) + } + + return stripped +} + +func stripViaDecoding(b []byte) ([]byte, error) { + var out bytes.Buffer + + decoder := yaml.NewDecoder(bytes.NewReader(b)) + encoder := yaml.NewEncoder(&out) + + for { + var node yaml.Node + + err := decoder.Decode(&node) + if err != nil { + if errors.Is(err, io.EOF) { + break + } + + return nil, err + } + + removeComments(&node) + + if err = encoder.Encode(&node); err != nil { + return nil, err + } + } + + return out.Bytes(), nil +} + +func removeComments(node *yaml.Node) { + node.FootComment = "" + node.HeadComment = "" + node.LineComment = "" + + for _, child := range node.Content { + removeComments(child) + } +} + +func stripManual(b []byte) []byte { + stripped := []byte{} + lines := bytes.Split(b, []byte("\n")) + + for i, line := range lines { + trimline := bytes.TrimSpace(line) + + // this is not accurate, but best effort + if bytes.HasPrefix(trimline, []byte("#")) && !bytes.HasPrefix(trimline, []byte("#!")) { + continue + } + + stripped = append(stripped, line...) + + if i < len(lines)-1 { + stripped = append(stripped, '\n') + } + } + + return stripped +} diff --git a/cmd/talosctl/pkg/talos/yamlstrip/yamlstrip_test.go b/cmd/talosctl/pkg/talos/yamlstrip/yamlstrip_test.go new file mode 100644 index 000000000..f73f9933e --- /dev/null +++ b/cmd/talosctl/pkg/talos/yamlstrip/yamlstrip_test.go @@ -0,0 +1,36 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package yamlstrip_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/siderolabs/talos/cmd/talosctl/pkg/talos/yamlstrip" +) + +func TestComments(t *testing.T) { + testCases, err := filepath.Glob(filepath.Join("testdata", "*.in.yaml")) + require.NoError(t, err) + + for _, path := range testCases { + path := path + + t.Run(filepath.Base(path), func(t *testing.T) { + in, err := os.ReadFile(path) + require.NoError(t, err) + + expected, err := os.ReadFile(strings.ReplaceAll(path, ".in.yaml", ".out.yaml")) + require.NoError(t, err) + + out := yamlstrip.Comments(in) + require.Equal(t, string(expected), string(out)) + }) + } +}