Merge pull request #4043 from phanimarupaka/PreserveIndentation

Make seq indent configurable and add retain seq indent functionality
This commit is contained in:
Kubernetes Prow Robot
2021-07-13 10:24:30 -07:00
committed by GitHub
11 changed files with 795 additions and 14 deletions

View File

@@ -37,6 +37,9 @@ type ByteReadWriter struct {
// the Resources, otherwise they will be cleared.
KeepReaderAnnotations bool
// PreserveSeqIndent if true adds kioutil.SeqIndentAnnotation to each resource
PreserveSeqIndent bool
// Style is a style that is set on the Resource Node Document.
Style yaml.Style
@@ -53,6 +56,7 @@ func (rw *ByteReadWriter) Read() ([]*yaml.RNode, error) {
b := &ByteReader{
Reader: rw.Reader,
OmitReaderAnnotations: rw.OmitReaderAnnotations,
PreserveSeqIndent: rw.PreserveSeqIndent,
}
val, err := b.Read()
if rw.FunctionConfig == nil {
@@ -109,9 +113,12 @@ type ByteReader struct {
Reader io.Reader
// OmitReaderAnnotations will configures Read to skip setting the config.kubernetes.io/index
// annotation on Resources as they are Read.
// and internal.config.kubernetes.io/seqindent annotations on Resources as they are Read.
OmitReaderAnnotations bool
// PreserveSeqIndent if true adds kioutil.SeqIndentAnnotation to each resource
PreserveSeqIndent bool
// SetAnnotations is a map of caller specified annotations to set on resources as they are read
// These are independent of the annotations controlled by OmitReaderAnnotations
SetAnnotations map[string]string
@@ -166,6 +173,10 @@ func splitDocuments(s string) ([]string, error) {
}
func (r *ByteReader) Read() ([]*yaml.RNode, error) {
if r.PreserveSeqIndent && r.OmitReaderAnnotations {
return nil, errors.Errorf(`"PreserveSeqIndent" option adds a reader annotation, please set "OmitReaderAnnotations" to false`)
}
output := ResourceNodeSlice{}
// by manually splitting resources -- otherwise the decoder will get the Resource
@@ -191,10 +202,11 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) {
values[i] += "\n"
}
decoder := yaml.NewDecoder(bytes.NewBufferString(values[i]))
node, err := r.decode(index, decoder)
node, err := r.decode(values[i], index, decoder)
if err == io.EOF {
continue
}
if err != nil {
return nil, errors.Wrap(err)
}
@@ -245,7 +257,7 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) {
return output, nil
}
func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, error) {
func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decoder) (*yaml.RNode, error) {
node := &yaml.Node{}
err := decoder.Decode(node)
if err == io.EOF {
@@ -268,6 +280,14 @@ func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, erro
}
if !r.OmitReaderAnnotations {
r.SetAnnotations[kioutil.IndexAnnotation] = fmt.Sprintf("%d", index)
if r.PreserveSeqIndent {
// derive and add the seqindent annotation
seqIndentStyle := yaml.DeriveSeqIndentStyle(originalYAML)
if seqIndentStyle != "" {
r.SetAnnotations[kioutil.SeqIndentAnnotation] = fmt.Sprintf("%s", seqIndentStyle)
}
}
}
var keys []string
for k := range r.SetAnnotations {

View File

@@ -10,6 +10,7 @@ import (
"github.com/stretchr/testify/assert"
. "sigs.k8s.io/kustomize/kyaml/kio"
"sigs.k8s.io/kustomize/kyaml/kio/kioutil"
)
func TestByteReader(t *testing.T) {
@@ -805,3 +806,102 @@ items:
})
}
}
// TestByteReader_AddSeqIndentAnnotation tests if the internal.config.kubernetes.io/seqindent
// annotation is added to resources appropriately
func TestByteReader_AddSeqIndentAnnotation(t *testing.T) {
type testCase struct {
name string
err string
input string
expectedAnnoValue string
OmitReaderAnnotations bool
}
testCases := []testCase{
{
name: "read with wide indentation",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
`,
expectedAnnoValue: "wide",
},
{
name: "read with compact indentation",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
`,
expectedAnnoValue: "compact",
},
{
name: "read with mixed indentation, wide wins",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
env:
- foo
- bar
`,
expectedAnnoValue: "wide",
},
{
name: "read with mixed indentation, compact wins",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
env:
- foo
- bar
`,
expectedAnnoValue: "compact",
},
{
name: "error if conflicting options",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
env:
- foo
- bar
`,
OmitReaderAnnotations: true,
err: `"PreserveSeqIndent" option adds a reader annotation, please set "OmitReaderAnnotations" to false`,
},
}
for i := range testCases {
tc := testCases[i]
t.Run(tc.name, func(t *testing.T) {
rNodes, err := (&ByteReader{
OmitReaderAnnotations: tc.OmitReaderAnnotations,
PreserveSeqIndent: true,
Reader: bytes.NewBuffer([]byte(tc.input)),
}).Read()
if tc.err != "" {
assert.Error(t, err)
assert.Equal(t, tc.err, err.Error())
return
}
assert.NoError(t, err)
actual := rNodes[0].GetAnnotations()[kioutil.SeqIndentAnnotation]
assert.Equal(t, tc.expectedAnnoValue, actual)
})
}
}

View File

@@ -324,3 +324,240 @@ functionConfig:
})
}
}
func TestByteReadWriter_RetainSeqIndent(t *testing.T) {
type testCase struct {
name string
err string
input string
expectedOutput string
instance kio.ByteReadWriter
}
testCases := []testCase{
{
name: "round_trip with 2 space seq indent",
input: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
---
apiVersion: v1
kind: Service
spec:
- foo
- bar
`,
expectedOutput: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
---
apiVersion: v1
kind: Service
spec:
- foo
- bar
`,
},
{
name: "round_trip with 0 space seq indent",
input: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
---
apiVersion: v1
kind: Service
spec:
- foo
- bar
`,
expectedOutput: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
---
apiVersion: v1
kind: Service
spec:
- foo
- bar
`,
},
{
name: "round_trip with different indentations",
input: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
---
apiVersion: v1
kind: Service
spec:
- foo
- bar
`,
expectedOutput: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
---
apiVersion: v1
kind: Service
spec:
- foo
- bar
`,
},
{
name: "round_trip with mixed indentations in same resource, wide wins as it is first",
input: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
env:
- foo
- bar
`,
expectedOutput: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
env:
- foo
- bar
`,
},
{
name: "round_trip with mixed indentations in same resource, compact wins as it is first",
input: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
env:
- foo
- bar
`,
expectedOutput: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
env:
- foo
- bar
`,
},
{
name: "unwrap ResourceList with annotations",
input: `
apiVersion: config.kubernetes.io/v1alpha1
kind: ResourceList
items:
- kind: Deployment
metadata:
annotations:
internal.config.kubernetes.io/seqindent: "compact"
spec:
- foo
- bar
- kind: Service
metadata:
annotations:
internal.config.kubernetes.io/seqindent: "wide"
spec:
- foo
- bar
`,
expectedOutput: `
kind: Deployment
spec:
- foo
- bar
---
kind: Service
spec:
- foo
- bar
`,
},
{
name: "round_trip with mixed indentations in same resource, wide wins as it is first",
input: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
env:
- foo
- bar
- baz
`,
expectedOutput: `
apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
env:
- foo
- bar
- baz
`,
},
}
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
nodes, err := w.Read()
if !assert.NoError(t, err) {
t.FailNow()
}
w.WrappingKind = ""
err = w.Write(nodes)
if !assert.NoError(t, err) {
t.FailNow()
}
if tc.err != "" {
if !assert.EqualError(t, err, tc.err) {
t.FailNow()
}
return
}
if !assert.Equal(t,
strings.TrimSpace(tc.expectedOutput), strings.TrimSpace(out.String())) {
t.FailNow()
}
})
}
}

View File

@@ -62,6 +62,12 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
// Even though we use the this value further down we must check this before removing annotations
jsonEncodeSingleBareNode := w.shouldJSONEncodeSingleBareNode(nodes)
// store seqindent annotation value for each node in order to set the encoder indentation
var seqIndentsForNodes []string
for i := range nodes {
seqIndentsForNodes = append(seqIndentsForNodes, nodes[i].GetAnnotations()[kioutil.SeqIndentAnnotation])
}
for i := range nodes {
// clean resources by removing annotations set by the Reader
if !w.KeepReaderAnnotations {
@@ -69,6 +75,11 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
if err != nil {
return errors.Wrap(err)
}
_, err = nodes[i].Pipe(yaml.ClearAnnotation(kioutil.SeqIndentAnnotation))
if err != nil {
return errors.Wrap(err)
}
}
for _, a := range w.ClearAnnotations {
_, err := nodes[i].Pipe(yaml.ClearAnnotation(a))
@@ -97,8 +108,13 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error {
// don't wrap the elements
if w.WrappingKind == "" {
for i := range nodes {
if seqIndentsForNodes[i] == string(yaml.WideSequenceStyle) {
encoder.DefaultSeqIndent()
} else {
encoder.CompactSeqIndent()
}
if err := encoder.Encode(nodes[i].Document()); err != nil {
return err
return errors.Wrap(err)
}
}
return nil

View File

@@ -311,6 +311,67 @@ metadata:
`,
},
//
// Test Case
//
{
name: "keep_annotation_seqindent",
instance: ByteWriter{KeepReaderAnnotations: true},
items: []string{
`a: b #first
metadata:
annotations:
config.kubernetes.io/index: 0
config.kubernetes.io/path: "a/b/a_test.yaml"
internal.config.kubernetes.io/index: "compact"
`,
`e: f
g:
h:
- i # has a list
- j
metadata:
annotations:
config.kubernetes.io/index: 0
config.kubernetes.io/path: "a/b/b_test.yaml"
internal.config.kubernetes.io/seqindent: "wide"
`,
`c: d # second
metadata:
annotations:
config.kubernetes.io/index: 1
config.kubernetes.io/path: "a/b/a_test.yaml"
internal.config.kubernetes.io/seqindent: "compact"
`,
},
expectedOutput: `a: b #first
metadata:
annotations:
config.kubernetes.io/index: 0
config.kubernetes.io/path: "a/b/a_test.yaml"
internal.config.kubernetes.io/index: "compact"
---
e: f
g:
h:
- i # has a list
- j
metadata:
annotations:
config.kubernetes.io/index: 0
config.kubernetes.io/path: "a/b/b_test.yaml"
internal.config.kubernetes.io/seqindent: "wide"
---
c: d # second
metadata:
annotations:
config.kubernetes.io/index: 1
config.kubernetes.io/path: "a/b/a_test.yaml"
internal.config.kubernetes.io/seqindent: "compact"
`,
},
//
// Test Case
//
@@ -337,6 +398,34 @@ metadata:
}`,
},
//
// Test Case
//
{
name: "encode_valid_json_remove_seqindent_annotation",
items: []string{
`{
"a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123",
metadata: {
annotations: {
"internal.config.kubernetes.io/seqindent": "compact",
"config.kubernetes.io/index": "0",
"config.kubernetes.io/path": "test.json"
}
}
}`,
},
expectedOutput: `{
"a": "a long string that would certainly see a newline introduced by the YAML marshaller abcd123",
"metadata": {
"annotations": {
"config.kubernetes.io/path": "test.json"
}
}
}`,
},
//
// Test Case
//

View File

@@ -22,6 +22,9 @@ const (
// PathAnnotation records the path to the file the Resource was read from
PathAnnotation AnnotationKey = "config.kubernetes.io/path"
// SeqIndentAnnotation records the sequence nodes indentation of the input resource
SeqIndentAnnotation AnnotationKey = "internal.config.kubernetes.io/seqindent"
)
func GetFileAnnotations(rn *yaml.RNode) (string, string, error) {

View File

@@ -40,6 +40,9 @@ type LocalPackageReadWriter struct {
KeepReaderAnnotations bool `yaml:"keepReaderAnnotations,omitempty"`
// PreserveSeqIndent if true adds kioutil.SeqIndentAnnotation to each resource
PreserveSeqIndent bool
// PackagePath is the path to the package directory.
PackagePath string `yaml:"path,omitempty"`
@@ -86,6 +89,7 @@ func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) {
SetAnnotations: r.SetAnnotations,
PackageFileName: r.PackageFileName,
FileSkipFunc: r.FileSkipFunc,
PreserveSeqIndent: r.PreserveSeqIndent,
}.Read()
if err != nil {
return nil, errors.Wrap(err)
@@ -177,6 +181,9 @@ type LocalPackageReader struct {
// FileSkipFunc is a function which returns true if reader should ignore
// the file
FileSkipFunc LocalPackageSkipFileFunc
// PreserveSeqIndent if true adds kioutil.SeqIndentAnnotation to each resource
PreserveSeqIndent bool
}
var _ Reader = LocalPackageReader{}
@@ -267,6 +274,7 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode
Reader: f,
OmitReaderAnnotations: r.OmitReaderAnnotations,
SetAnnotations: r.SetAnnotations,
PreserveSeqIndent: r.PreserveSeqIndent,
}
return rr.Read()
}

View File

@@ -337,6 +337,69 @@ g:
}
}
func TestLocalPackageReader_Read_PreserveSeqIndent(t *testing.T) {
s := SetupDirectories(t, filepath.Join("a", "b"), filepath.Join("a", "c"))
defer s.Clean()
s.WriteFile(t, filepath.Join("a_test.yaml"), readFileA)
s.WriteFile(t, filepath.Join("b_test.yaml"), readFileB)
paths := []struct {
path string
}{
{path: "./"},
{path: s.Root},
}
for _, p := range paths {
// empty path
rfr := LocalPackageReader{PackagePath: p.path, PreserveSeqIndent: true}
nodes, err := rfr.Read()
if !assert.NoError(t, err) {
return
}
if !assert.Len(t, nodes, 3) {
return
}
expected := []string{
`a: b #first
metadata:
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/path: 'a_test.yaml'
internal.config.kubernetes.io/seqindent: 'compact'
`,
`c: d # second
metadata:
annotations:
config.kubernetes.io/index: '1'
config.kubernetes.io/path: 'a_test.yaml'
internal.config.kubernetes.io/seqindent: 'compact'
`,
`# second thing
e: f
g:
h:
- i # has a list
- j
metadata:
annotations:
config.kubernetes.io/index: '0'
config.kubernetes.io/path: 'b_test.yaml'
internal.config.kubernetes.io/seqindent: 'compact'
`,
}
for i := range nodes {
val, err := nodes[i].String()
if !assert.NoError(t, err) {
return
}
if !assert.Equal(t, expected[i], val) {
return
}
}
}
}
func TestLocalPackageReader_Read_nestedDirs(t *testing.T) {
s := SetupDirectories(t, filepath.Join("a", "b"), filepath.Join("a", "c"))
defer s.Clean()

View File

@@ -10,14 +10,21 @@ import (
"sigs.k8s.io/kustomize/kyaml/internal/forked/github.com/go-yaml/yaml"
)
const CompactSequenceStyle = "compact"
const WideSequenceStyle = "wide"
const (
WideSequenceStyle SequenceIndentStyle = "wide"
CompactSequenceStyle SequenceIndentStyle = "compact"
DefaultIndent = 2
)
const DefaultIndent = 2
const DefaultSequenceStyle = CompactSequenceStyle
// SeqIndentType holds the indentation style for sequence nodes
type SequenceIndentStyle string
var sequenceIndentationStyle = DefaultSequenceStyle
var indent = DefaultIndent
// EncoderOptions are options that can be used to configure the encoder,
// do not expose new options without considerable justification
type EncoderOptions struct {
// SeqIndent is the indentation style for YAML Sequence nodes
SeqIndent SequenceIndentStyle
}
// Expose the yaml.v3 functions so this package can be used as a replacement
@@ -43,13 +50,33 @@ var Unmarshal = yaml.Unmarshal
var NewDecoder = yaml.NewDecoder
var NewEncoder = func(w io.Writer) *yaml.Encoder {
e := yaml.NewEncoder(w)
e.SetIndent(indent)
if sequenceIndentationStyle == CompactSequenceStyle {
e.CompactSeqIndent()
}
e.SetIndent(DefaultIndent)
e.CompactSeqIndent()
return e
}
// MarshalWithOptions marshals the input interface with provided options
func MarshalWithOptions(in interface{}, opts *EncoderOptions) ([]byte, error) {
var buf bytes.Buffer
err := NewEncoderWithOptions(&buf, opts).Encode(in)
if err != nil {
return nil, err
}
return buf.Bytes(), nil
}
// NewEncoderWithOptions returns the encoder with provided options
func NewEncoderWithOptions(w io.Writer, opts *EncoderOptions) *yaml.Encoder {
encoder := NewEncoder(w)
encoder.SetIndent(DefaultIndent)
if opts.SeqIndent == WideSequenceStyle {
encoder.DefaultSeqIndent()
} else {
encoder.CompactSeqIndent()
}
return encoder
}
var AliasNode yaml.Kind = yaml.AliasNode
var DocumentNode yaml.Kind = yaml.DocumentNode
var MappingNode yaml.Kind = yaml.MappingNode

49
kyaml/yaml/util.go Normal file
View File

@@ -0,0 +1,49 @@
package yaml
import (
"strings"
)
// DeriveSeqIndentStyle derives the sequence indentation annotation value for the resource,
// originalYAML is the input yaml string,
// the style is decided by deriving the existing sequence indentation of first sequence node
func DeriveSeqIndentStyle(originalYAML string) string {
lines := strings.Split(originalYAML, "\n")
for i, line := range lines {
elems := strings.SplitN(line, "- ", 2)
if len(elems) != 2 {
continue
}
// prefix of "- " must be sequence of spaces
if strings.Trim(elems[0], " ") != "" {
continue
}
numSpacesBeforeSeqElem := len(elems[0])
if i == 0 {
continue
}
// keyLine is the line before the first sequence element
keyLine := lines[i-1]
numSpacesBeforeKeyElem := len(keyLine) - len(strings.TrimLeft(keyLine, " "))
trimmedKeyLine := strings.Trim(keyLine, " ")
if strings.Count(trimmedKeyLine, ":") != 1 || !strings.HasSuffix(trimmedKeyLine, ":") {
// if the key line doesn't contain only one : that too at the end,
// this is not a sequence node, it is a wrapped sequence node string
// ignore it
continue
}
if numSpacesBeforeSeqElem == numSpacesBeforeKeyElem {
return string(CompactSequenceStyle)
}
if numSpacesBeforeSeqElem-numSpacesBeforeKeyElem == 2 {
return string(WideSequenceStyle)
}
}
return string(CompactSequenceStyle)
}

169
kyaml/yaml/util_test.go Normal file
View File

@@ -0,0 +1,169 @@
package yaml
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestDeriveSeqIndentStyle(t *testing.T) {
type testCase struct {
name string
input string
expectedOutput string
}
testCases := []testCase{
{
name: "detect simple wide indent",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
`,
expectedOutput: `wide`,
},
{
name: "detect simple compact indent",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
`,
expectedOutput: `compact`,
},
{
name: "read with mixed indentation, wide first",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
env:
- foo
- bar
`,
expectedOutput: `wide`,
},
{
name: "read with mixed indentation, compact first",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
- baz
env:
- foo
- bar
`,
expectedOutput: `compact`,
},
{
name: "read with mixed indentation, compact first with less elements",
input: `apiVersion: apps/v1
kind: Deployment
spec:
- foo
- bar
env:
- foo
- bar
- baz
`,
expectedOutput: `compact`,
},
{
name: "skip wrapped sequence strings, pipe hyphen",
input: `apiVersion: apps/v1
kind: Deployment
spec: |-
- foo
- bar
`,
expectedOutput: `compact`,
},
{
name: "skip wrapped sequence strings, pipe",
input: `apiVersion: apps/v1
kind: Deployment
spec: |
- foo
- bar
`,
expectedOutput: `compact`,
},
{
name: "skip wrapped sequence strings, right angle bracket",
input: `apiVersion: apps/v1
kind: Deployment
spec: >
- foo
- bar
`,
expectedOutput: `compact`,
},
{
name: "skip wrapped sequence strings, plus",
input: `apiVersion: apps/v1
kind: Deployment
spec: +
- foo
- bar
`,
expectedOutput: `compact`,
},
{
name: "nested wide vs compact",
input: `apiVersion: apps/v1
kind: Deployment
spec:
foo:
bar:
baz:
bor:
- a
- b
abc:
- a
- b
`,
expectedOutput: `wide`,
},
{
name: "invalid resource but valid yaml sequence",
input: ` - foo`,
expectedOutput: `compact`,
},
{
name: "- within sequence element",
input: `apiVersion: apps/v1
kind: Deployment
spec:
foo:
- - a`,
expectedOutput: `wide`,
},
{
name: "- within non sequence element",
input: `apiVersion: apps/v1
kind: Deployment
spec:
foo:
a: - b`,
expectedOutput: `compact`,
},
}
for i := range testCases {
tc := testCases[i]
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.expectedOutput, DeriveSeqIndentStyle(tc.input))
})
}
}