From ed38b5fe2b1de21cd5b83cff3e7d439588e8148a Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Wed, 7 Jul 2021 15:33:09 -0700 Subject: [PATCH] Make seq indent configurable and add retain seq indent functionality --- kyaml/kio/byteio_reader.go | 68 ++++++- kyaml/kio/byteio_reader_test.go | 119 ++++++++++++ kyaml/kio/byteio_readwriter_test.go | 284 ++++++++++++++++++++++++++++ kyaml/kio/byteio_writer.go | 20 ++ kyaml/kio/kioutil/kioutil.go | 3 + kyaml/kio/pkgio_reader.go | 11 +- kyaml/yaml/alias.go | 10 + 7 files changed, 509 insertions(+), 6 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 271b26d22..fd423120d 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -11,6 +11,7 @@ import ( "sort" "strings" + "sigs.k8s.io/kustomize/kyaml/copyutil" "sigs.k8s.io/kustomize/kyaml/errors" "sigs.k8s.io/kustomize/kyaml/kio/kioutil" "sigs.k8s.io/kustomize/kyaml/yaml" @@ -47,12 +48,16 @@ type ByteReadWriter struct { NoWrap bool WrappingAPIVersion string WrappingKind string + + // RetainSeqIndent if true retains the sequence indentation of + RetainSeqIndent bool } func (rw *ByteReadWriter) Read() ([]*yaml.RNode, error) { b := &ByteReader{ - Reader: rw.Reader, - OmitReaderAnnotations: rw.OmitReaderAnnotations, + Reader: rw.Reader, + OmitReaderAnnotations: rw.OmitReaderAnnotations, + AddSeqIndentAnnotation: rw.RetainSeqIndent, } val, err := b.Read() if rw.FunctionConfig == nil { @@ -130,6 +135,9 @@ 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 + + // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource + AddSeqIndentAnnotation bool } var _ Reader = &ByteReader{} @@ -195,6 +203,13 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) { if err == io.EOF { continue } + + if r.AddSeqIndentAnnotation { + if err := addSeqIndentAnno(values[i], node); err != nil { + return nil, errors.Wrap(err) + } + } + if err != nil { return nil, errors.Wrap(err) } @@ -245,6 +260,55 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) { return output, nil } +// addSeqIndentAnno adds the sequence indentation annotation to the resource +// value is the input yaml string and node is the decoded equivalent of value +// the annotation value is decided by deriving the existing sequence indentation of resource +func addSeqIndentAnno(value string, node *yaml.RNode) error { + anno := node.GetAnnotations() + if anno[kioutil.SeqIndentAnnotation] != "" { + // the annotation already exists, so don't change it + return nil + } + + currentDefaultIndent := yaml.SequenceIndentationStyle() + defer yaml.SetSequenceIndentationStyle(currentDefaultIndent) + + // encode the node to string with default 2 space sequence indentation and calculate the diff + yaml.SetSequenceIndentationStyle(yaml.WideSequenceStyle) + n, err := yaml.Parse(value) + if err != nil { + return err + } + out, err := n.String() + if err != nil { + return err + } + twoSpaceIndentDiff := copyutil.PrettyFileDiff(out, value) + + // encode the node to string with compact 0 space sequence indentation and calculate the diff + yaml.SetSequenceIndentationStyle(yaml.CompactSequenceStyle) + n, err = yaml.Parse(value) + if err != nil { + return err + } + out, err = n.String() + if err != nil { + return err + } + noIndentDiff := copyutil.PrettyFileDiff(out, value) + + var style string + if len(noIndentDiff) <= len(twoSpaceIndentDiff) { + style = yaml.CompactSequenceStyle + } else { + style = yaml.WideSequenceStyle + } + + return node.PipeE( + yaml.LookupCreate(yaml.MappingNode, yaml.MetadataField, yaml.AnnotationsField), + yaml.SetField(kioutil.SeqIndentAnnotation, yaml.NewScalarRNode(style))) +} + func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, error) { node := &yaml.Node{} err := decoder.Decode(node) diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index 2d04e0a6b..3d9bbdcc8 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/kyaml/kio/byteio_reader_test.go @@ -805,3 +805,122 @@ items: }) } } + +func TestByteReader_RetainSeqIndent(t *testing.T) { + type testCase struct { + name string + err string + input string + expectedOutput string + } + + testCases := []testCase{ + { + name: "read with wide indentation", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar + - baz +`, + expectedOutput: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +metadata: + annotations: + internal.config.kubernetes.io/seqindent: wide +`, + }, + { + name: "read with compact indentation", + input: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +`, + expectedOutput: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +metadata: + annotations: + internal.config.kubernetes.io/seqindent: compact +`, + }, + { + name: "read with mixed indentation, wide wins", + input: `apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar + - baz +env: +- foo +- bar +`, + expectedOutput: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +env: +- foo +- bar +metadata: + annotations: + internal.config.kubernetes.io/seqindent: wide +`, + }, + { + name: "read with mixed indentation, compact wins", + input: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +env: + - foo + - bar +`, + expectedOutput: `apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +env: +- foo +- bar +metadata: + annotations: + internal.config.kubernetes.io/seqindent: compact +`, + }, + } + + for i := range testCases { + tc := testCases[i] + t.Run(tc.name, func(t *testing.T) { + rNodes, err := (&ByteReader{ + OmitReaderAnnotations: true, + AddSeqIndentAnnotation: true, + Reader: bytes.NewBuffer([]byte(tc.input)), + }).Read() + assert.NoError(t, err) + actual, err := rNodes[0].String() + assert.NoError(t, err) + assert.Equal(t, tc.expectedOutput, actual) + }) + } +} diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 0e965186a..fe7490fa3 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -289,6 +289,47 @@ functionConfig: `, instance: kio.ByteReadWriter{FunctionConfig: yaml.MustParse(`c: d`)}, }, + { + name: "ResourceList indentation doesn't matter", + 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: ` +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 +`, + }, } for i := range testCases { @@ -324,3 +365,246 @@ 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, least diff wins", + input: ` +apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar + - baz +env: +- foo +- bar +`, + expectedOutput: ` +apiVersion: apps/v1 +kind: Deployment +spec: + - foo + - bar + - baz +env: + - foo + - bar +`, + }, + { + name: "round_trip with mixed indentations in same resource, least diff wins", + input: ` +apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +env: + - foo + - bar +`, + expectedOutput: ` +apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +- baz +env: +- foo +- bar +`, + }, + { + name: "round_trip with mixed indentations in same resource, compact in case of a tie", + input: ` +apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +env: + - foo + - bar +`, + expectedOutput: ` +apiVersion: apps/v1 +kind: Deployment +spec: +- foo +- bar +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 +`, + }, + } + + 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.RetainSeqIndent = 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() + } + }) + } +} diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index a208f52bb..3d8303935 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -97,6 +97,7 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { // don't wrap the elements if w.WrappingKind == "" { for i := range nodes { + setSeqIndent(nodes[i], encoder) if err := encoder.Encode(nodes[i].Document()); err != nil { return err } @@ -134,6 +135,25 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { return encoder.Encode(doc) } +// setSeqIndent reads the SeqIndentAnnotation from node and sets the sequence indentation +// in the encoder +// if the resource doesn't have SeqIndentAnnotation it will use CompactSeqIndent +func setSeqIndent(node *yaml.RNode, encoder *yaml.Encoder) { + anno := node.GetAnnotations() + seqIndent := anno[kioutil.SeqIndentAnnotation] + if seqIndent == yaml.WideSequenceStyle { + encoder.DefaultSeqIndent() + } else { + encoder.CompactSeqIndent() + } + if err := node.PipeE(yaml.ClearAnnotation(kioutil.SeqIndentAnnotation)); err != nil { + return + } + if err := yaml.ClearEmptyAnnotations(node); err != nil { + return + } +} + func copyRNodes(in []*yaml.RNode) []*yaml.RNode { out := make([]*yaml.RNode, len(in)) for i := range in { diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go index 1d5e3bf58..8d599448c 100644 --- a/kyaml/kio/kioutil/kioutil.go +++ b/kyaml/kio/kioutil/kioutil.go @@ -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 path to the file the Resource was read from + SeqIndentAnnotation AnnotationKey = "internal.config.kubernetes.io/seqindent" ) func GetFileAnnotations(rn *yaml.RNode) (string, string, error) { diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index 1dfec3c77..49b8457df 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -177,6 +177,8 @@ type LocalPackageReader struct { // FileSkipFunc is a function which returns true if reader should ignore // the file FileSkipFunc LocalPackageSkipFileFunc + + RetainSeqIndent bool } var _ Reader = LocalPackageReader{} @@ -263,10 +265,11 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode defer f.Close() rr := &ByteReader{ - DisableUnwrapping: true, - Reader: f, - OmitReaderAnnotations: r.OmitReaderAnnotations, - SetAnnotations: r.SetAnnotations, + DisableUnwrapping: true, + Reader: f, + OmitReaderAnnotations: r.OmitReaderAnnotations, + SetAnnotations: r.SetAnnotations, + AddSeqIndentAnnotation: r.RetainSeqIndent, } return rr.Read() } diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 05cafae0d..8ce609fa9 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -19,6 +19,16 @@ const DefaultSequenceStyle = CompactSequenceStyle var sequenceIndentationStyle = DefaultSequenceStyle var indent = DefaultIndent +// SetSequenceIndentationStyle sets the sequenceIndentationStyle variable +func SetSequenceIndentationStyle(style string) { + sequenceIndentationStyle = style +} + +// SequenceIndentationStyle returns the value of sequenceIndentationStyle +func SequenceIndentationStyle() string { + return sequenceIndentationStyle +} + // Expose the yaml.v3 functions so this package can be used as a replacement type Decoder = yaml.Decoder