From e997cc5486e089a0c3c8e0e33f81cb4b439d420e Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Fri, 17 Sep 2021 13:40:56 -0700 Subject: [PATCH] Handle parsing of bare sequence yaml nodes --- kyaml/kio/byteio_reader.go | 31 +++++- kyaml/kio/byteio_readwriter_test.go | 150 ++++++++++++++++++++++++++++ kyaml/kio/byteio_writer.go | 13 ++- kyaml/kio/pkgio_reader.go | 16 +++ kyaml/kio/pkgio_reader_test.go | 45 +++++++++ kyaml/yaml/alias.go | 4 + 6 files changed, 256 insertions(+), 3 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 7513f0d64..e3fcb585b 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -43,6 +43,13 @@ type ByteReadWriter struct { // Style is a style that is set on the Resource Node Document. Style yaml.Style + // WrapBareSeqNode wraps the bare sequence node document with map node, + // kyaml uses reader annotations to track resources, it is not possible to + // add them to bare sequence nodes, this option enables wrapping such bare + // sequence nodes into map node with key yaml.BareSeqNodeWrappingKey + // note that this wrapping is different and not related to ResourceList wrapping + WrapBareSeqNode bool + FunctionConfig *yaml.RNode Results *yaml.RNode @@ -57,6 +64,7 @@ func (rw *ByteReadWriter) Read() ([]*yaml.RNode, error) { Reader: rw.Reader, OmitReaderAnnotations: rw.OmitReaderAnnotations, PreserveSeqIndent: rw.PreserveSeqIndent, + WrapBareSeqNode: rw.WrapBareSeqNode, } val, err := b.Read() if rw.FunctionConfig == nil { @@ -137,6 +145,13 @@ type ByteReader struct { // WrappingKind is set by Read(), and is the kind of the object that // the read objects were originally wrapped in. WrappingKind string + + // WrapBareSeqNode wraps the bare sequence node document with map node, + // kyaml uses reader annotations to track resources, it is not possible to + // add them to bare sequence nodes, this option enables wrapping such bare + // sequence nodes into map node with key yaml.BareSeqNodeWrappingKey + // note that this wrapping is different and not related to ResourceList wrapping + WrapBareSeqNode bool } var _ Reader = &ByteReader{} @@ -208,6 +223,10 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) { } if err != nil { + if r.WrapBareSeqNode { + // continue reading other resources if failed to parse a resource + continue + } return nil, errors.Wrap(err) } if yaml.IsMissingOrNull(node) { @@ -275,6 +294,14 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode // sort the annotations by key so the output Resources is consistent (otherwise the // annotations will be in a random order) n := yaml.NewRNode(node) + wrappedNode := yaml.NewRNode(&yaml.Node{ + Kind: yaml.MappingNode, + }) + if r.WrapBareSeqNode && node.Kind == yaml.DocumentNode && node.Content[0].Kind == yaml.SequenceNode { + wrappedNode.PipeE(yaml.SetField(yaml.BareSeqNodeWrappingKey, n)) + } else { + wrappedNode = n + } if r.SetAnnotations == nil { r.SetAnnotations = map[string]string{} } @@ -295,10 +322,10 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode } sort.Strings(keys) for _, k := range keys { - _, err = n.Pipe(yaml.SetAnnotation(k, r.SetAnnotations[k])) + _, err = wrappedNode.Pipe(yaml.SetAnnotation(k, r.SetAnnotations[k])) if err != nil { return nil, errors.Wrap(err) } } - return yaml.NewRNode(node), nil + return wrappedNode, nil } diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 9b6deada4..5eba929e8 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -589,3 +589,153 @@ env: }) } } + +func TestByteReadWriter_WrapBareSeqNode(t *testing.T) { + type testCase struct { + name string + readerErr string + writerErr string + input string + wrapBareSeqNode bool + expectedOutput string + instance kio.ByteReadWriter + } + + testCases := []testCase{ + { + name: "round_trip bare seq node simple", + wrapBareSeqNode: true, + input: ` +- foo +- bar +`, + expectedOutput: ` +- foo +- bar +`, + }, + { + name: "round_trip bare seq node", + wrapBareSeqNode: true, + input: `# Use the old CRD because of the quantity validation issue: +# https://github.com/kubeflow/kubeflow/issues/5722 +- op: replace + path: /spec + value: + group: kubeflow.org + names: + kind: Notebook + plural: notebooks + singular: notebook + scope: Namespaced + subresources: + status: {} + versions: + - name: v1alpha1 + served: true + storage: false +`, + expectedOutput: `# Use the old CRD because of the quantity validation issue: +# https://github.com/kubeflow/kubeflow/issues/5722 +- op: replace + path: /spec + value: + group: kubeflow.org + names: + kind: Notebook + plural: notebooks + singular: notebook + scope: Namespaced + subresources: + status: {} + versions: + - name: v1alpha1 + served: true + storage: false +`, + }, + { + name: "error round_trip bare seq node simple", + wrapBareSeqNode: false, + input: ` +- foo +- bar +`, + readerErr: "wrong Node Kind for expected: MappingNode was SequenceNode", + }, + { + name: "error k fround_trip bare seq node", + wrapBareSeqNode: false, + input: `# Use the old CRD because of the quantity validation issue: +# https://github.com/kubeflow/kubeflow/issues/5722 +- op: replace + path: /spec + value: + group: kubeflow.org + names: + kind: Notebook + plural: notebooks + singular: notebook + scope: Namespaced + subresources: + status: {} + versions: + - name: v1alpha1 + served: true + storage: false +`, + readerErr: "wrong Node Kind for expected: MappingNode was SequenceNode", + }, + { + name: "round_trip bare seq node json", + wrapBareSeqNode: true, + input: `[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--namespaced"}]`, + expectedOutput: `[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--namespaced"}]`, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + var in, out bytes.Buffer + in.WriteString(tc.input) + w := tc.instance + w.Writer = &out + w.Reader = &in + w.PreserveSeqIndent = true + w.WrapBareSeqNode = tc.wrapBareSeqNode + + nodes, err := w.Read() + if tc.readerErr != "" { + if !assert.Error(t, err) { + t.FailNow() + } + if !assert.Contains(t, err.Error(), tc.readerErr) { + t.FailNow() + } + return + } + + w.WrappingKind = "" + err = w.Write(nodes) + if !assert.NoError(t, err) { + t.FailNow() + } + + if tc.writerErr != "" { + if !assert.Error(t, err) { + t.FailNow() + } + if !assert.Contains(t, err.Error(), tc.writerErr) { + t.FailNow() + } + return + } + + if !assert.Equal(t, + strings.TrimSpace(tc.expectedOutput), strings.TrimSpace(out.String())) { + t.FailNow() + } + }) + } +} diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index 33e5a8999..9bd57b72a 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -7,6 +7,7 @@ import ( "encoding/json" "io" "path/filepath" + "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -113,7 +114,7 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { } else { encoder.CompactSeqIndent() } - if err := encoder.Encode(nodes[i].Document()); err != nil { + if err := encoder.Encode(upWrapBareSequenceNode(nodes[i].Document())); err != nil { return errors.Wrap(err) } } @@ -181,3 +182,13 @@ func (w ByteWriter) shouldJSONEncodeSingleBareNode(nodes []*yaml.RNode) bool { } return false } + +// upWrapBareSequenceNode unwraps the bare sequence nodes wrapped by yaml.BareSeqNodeWrappingKey +func upWrapBareSequenceNode(node *yaml.Node) *yaml.Node { + rNode := yaml.NewRNode(node) + seqNode, err := rNode.Pipe(yaml.Lookup(yaml.BareSeqNodeWrappingKey)) + if err == nil && !seqNode.IsNilOrEmpty() { + return seqNode.YNode() + } + return node +} diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index e36107787..b5d67c393 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -82,6 +82,13 @@ type LocalPackageReadWriter struct { // FileSystem can be used to mock the disk file system. FileSystem filesys.FileSystemOrOnDisk + + // WrapBareSeqNode wraps the bare sequence node document with map node, + // kyaml uses reader annotations to track resources, it is not possible to + // add them to bare sequence nodes, this option enables wrapping such bare + // sequence nodes into map node with key yaml.BareSeqNodeWrappingKey + // note that this wrapping is different and not related to ResourceList wrapping + WrapBareSeqNode bool } func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) { @@ -95,6 +102,7 @@ func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) { FileSkipFunc: r.FileSkipFunc, PreserveSeqIndent: r.PreserveSeqIndent, FileSystem: r.FileSystem, + WrapBareSeqNode: r.WrapBareSeqNode, }.Read() if err != nil { return nil, errors.Wrap(err) @@ -193,6 +201,13 @@ type LocalPackageReader struct { // FileSystem can be used to mock the disk file system. FileSystem filesys.FileSystemOrOnDisk + + // WrapBareSeqNode wraps the bare sequence node document with map node, + // kyaml uses reader annotations to track resources, it is not possible to + // add them to bare sequence nodes, this option enables wrapping such bare + // sequence nodes into map node with key yaml.BareSeqNodeWrappingKey + // note that this wrapping is different and not related to ResourceList wrapping + WrapBareSeqNode bool } var _ Reader = LocalPackageReader{} @@ -287,6 +302,7 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode OmitReaderAnnotations: r.OmitReaderAnnotations, SetAnnotations: r.SetAnnotations, PreserveSeqIndent: r.PreserveSeqIndent, + WrapBareSeqNode: r.WrapBareSeqNode, } return rr.Read() } diff --git a/kyaml/kio/pkgio_reader_test.go b/kyaml/kio/pkgio_reader_test.go index a6df5ec10..c8cacfe34 100644 --- a/kyaml/kio/pkgio_reader_test.go +++ b/kyaml/kio/pkgio_reader_test.go @@ -39,6 +39,13 @@ a: b #forth metadata: `) +var readFileE = []byte(`--- +a: b #first +--- +- foo # second +- bar +`) + var pkgFile = []byte(``) func TestLocalPackageReader_Read_empty(t *testing.T) { @@ -555,3 +562,41 @@ func testOnDiskAndOnMem(t *testing.T, files []mockFile, f func(t *testing.T, pat f(t, "/", fs) }) } + +func TestLocalPackageReader_ReadBareSeqNodes(t *testing.T) { + testOnDiskAndOnMem(t, []mockFile{ + {path: "a/b"}, + {path: "a/c"}, + {path: "e_test.yaml", content: readFileE}, + }, func(t *testing.T, path string, mockFS filesys.FileSystem) { + rfr := LocalPackageReader{ + PackagePath: path, + FileSystem: filesys.FileSystemOrOnDisk{FileSystem: mockFS}, + WrapBareSeqNode: true, + } + nodes, err := rfr.Read() + require.NoError(t, err) + require.Len(t, nodes, 2) + expected := []string{ + `a: b #first +metadata: + annotations: + config.kubernetes.io/index: '0' + config.kubernetes.io/path: 'e_test.yaml' +`, + `bareSeqNodeWrappingKey: +- foo # second +- bar +metadata: + annotations: + config.kubernetes.io/index: '1' + config.kubernetes.io/path: 'e_test.yaml' +`, + } + for i := range nodes { + val, err := nodes[i].String() + require.NoError(t, err) + require.Equal(t, expected[i], val) + } + }) +} diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 5a3730118..198171875 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -14,6 +14,10 @@ const ( WideSequenceStyle SequenceIndentStyle = "wide" CompactSequenceStyle SequenceIndentStyle = "compact" DefaultIndent = 2 + // BareSeqNodeWrappingKey kyaml uses reader annotations to track resources, it is not possible to + // add them to bare sequence nodes, this key is used to wrap such bare + // sequence nodes into map node, byteio_writer unwraps it while writing back + BareSeqNodeWrappingKey = "bareSeqNodeWrappingKey" ) // SeqIndentType holds the indentation style for sequence nodes