Handle parsing of bare sequence yaml nodes

This commit is contained in:
Phani Teja Marupaka
2021-09-17 13:40:56 -07:00
parent 53577a5190
commit e997cc5486
6 changed files with 256 additions and 3 deletions

View File

@@ -43,6 +43,13 @@ type ByteReadWriter struct {
// Style is a style that is set on the Resource Node Document. // Style is a style that is set on the Resource Node Document.
Style yaml.Style 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 FunctionConfig *yaml.RNode
Results *yaml.RNode Results *yaml.RNode
@@ -57,6 +64,7 @@ func (rw *ByteReadWriter) Read() ([]*yaml.RNode, error) {
Reader: rw.Reader, Reader: rw.Reader,
OmitReaderAnnotations: rw.OmitReaderAnnotations, OmitReaderAnnotations: rw.OmitReaderAnnotations,
PreserveSeqIndent: rw.PreserveSeqIndent, PreserveSeqIndent: rw.PreserveSeqIndent,
WrapBareSeqNode: rw.WrapBareSeqNode,
} }
val, err := b.Read() val, err := b.Read()
if rw.FunctionConfig == nil { if rw.FunctionConfig == nil {
@@ -137,6 +145,13 @@ type ByteReader struct {
// WrappingKind is set by Read(), and is the kind of the object that // WrappingKind is set by Read(), and is the kind of the object that
// the read objects were originally wrapped in. // the read objects were originally wrapped in.
WrappingKind string 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{} var _ Reader = &ByteReader{}
@@ -208,6 +223,10 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) {
} }
if err != nil { if err != nil {
if r.WrapBareSeqNode {
// continue reading other resources if failed to parse a resource
continue
}
return nil, errors.Wrap(err) return nil, errors.Wrap(err)
} }
if yaml.IsMissingOrNull(node) { 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 // sort the annotations by key so the output Resources is consistent (otherwise the
// annotations will be in a random order) // annotations will be in a random order)
n := yaml.NewRNode(node) 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 { if r.SetAnnotations == nil {
r.SetAnnotations = map[string]string{} r.SetAnnotations = map[string]string{}
} }
@@ -295,10 +322,10 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode
} }
sort.Strings(keys) sort.Strings(keys)
for _, k := range 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 { if err != nil {
return nil, errors.Wrap(err) return nil, errors.Wrap(err)
} }
} }
return yaml.NewRNode(node), nil return wrappedNode, nil
} }

View File

@@ -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()
}
})
}
}

View File

@@ -7,6 +7,7 @@ import (
"encoding/json" "encoding/json"
"io" "io"
"path/filepath" "path/filepath"
"sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/errors"
"sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/kio/kioutil"
"sigs.k8s.io/kustomize/kyaml/yaml" "sigs.k8s.io/kustomize/kyaml/yaml"
@@ -113,7 +114,7 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
} else { } else {
encoder.CompactSeqIndent() 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) return errors.Wrap(err)
} }
} }
@@ -181,3 +182,13 @@ func (w ByteWriter) shouldJSONEncodeSingleBareNode(nodes []*yaml.RNode) bool {
} }
return false 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
}

View File

@@ -82,6 +82,13 @@ type LocalPackageReadWriter struct {
// FileSystem can be used to mock the disk file system. // FileSystem can be used to mock the disk file system.
FileSystem filesys.FileSystemOrOnDisk 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) { func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) {
@@ -95,6 +102,7 @@ func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) {
FileSkipFunc: r.FileSkipFunc, FileSkipFunc: r.FileSkipFunc,
PreserveSeqIndent: r.PreserveSeqIndent, PreserveSeqIndent: r.PreserveSeqIndent,
FileSystem: r.FileSystem, FileSystem: r.FileSystem,
WrapBareSeqNode: r.WrapBareSeqNode,
}.Read() }.Read()
if err != nil { if err != nil {
return nil, errors.Wrap(err) return nil, errors.Wrap(err)
@@ -193,6 +201,13 @@ type LocalPackageReader struct {
// FileSystem can be used to mock the disk file system. // FileSystem can be used to mock the disk file system.
FileSystem filesys.FileSystemOrOnDisk 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{} var _ Reader = LocalPackageReader{}
@@ -287,6 +302,7 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode
OmitReaderAnnotations: r.OmitReaderAnnotations, OmitReaderAnnotations: r.OmitReaderAnnotations,
SetAnnotations: r.SetAnnotations, SetAnnotations: r.SetAnnotations,
PreserveSeqIndent: r.PreserveSeqIndent, PreserveSeqIndent: r.PreserveSeqIndent,
WrapBareSeqNode: r.WrapBareSeqNode,
} }
return rr.Read() return rr.Read()
} }

View File

@@ -39,6 +39,13 @@ a: b #forth
metadata: metadata:
`) `)
var readFileE = []byte(`---
a: b #first
---
- foo # second
- bar
`)
var pkgFile = []byte(``) var pkgFile = []byte(``)
func TestLocalPackageReader_Read_empty(t *testing.T) { 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) 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)
}
})
}

View File

@@ -14,6 +14,10 @@ const (
WideSequenceStyle SequenceIndentStyle = "wide" WideSequenceStyle SequenceIndentStyle = "wide"
CompactSequenceStyle SequenceIndentStyle = "compact" CompactSequenceStyle SequenceIndentStyle = "compact"
DefaultIndent = 2 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 // SeqIndentType holds the indentation style for sequence nodes