From ed38b5fe2b1de21cd5b83cff3e7d439588e8148a Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Wed, 7 Jul 2021 15:33:09 -0700 Subject: [PATCH 1/7] 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 271b26d220..fd423120d1 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 2d04e0a6b9..3d9bbdcc83 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 0e965186ad..fe7490fa3d 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -289,6 +289,288 @@ 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 { + 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 + + nodes, err := w.Read() + if !assert.NoError(t, err) { + t.FailNow() + } + + 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() + } + }) + } +} + +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 { @@ -299,12 +581,14 @@ functionConfig: 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() diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index a208f52bb7..3d83039358 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 1d5e3bf586..8d599448c6 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 1dfec3c773..49b8457df1 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 05cafae0d8..8ce609fa98 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 From 6dbc74b32e8c23b7a5d2d546b50902a050f33fcb Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Wed, 7 Jul 2021 23:22:15 -0700 Subject: [PATCH 2/7] Suggested changes --- kyaml/kio/byteio_reader.go | 45 ++++++++++--------------- kyaml/kio/byteio_reader_test.go | 2 +- kyaml/kio/byteio_readwriter_test.go | 2 +- kyaml/kio/byteio_writer.go | 24 ++++++------- kyaml/kio/kioutil/kioutil.go | 2 +- kyaml/kio/pkgio_reader.go | 5 +-- kyaml/yaml/alias.go | 52 +++++++++++++++++------------ kyaml/yaml/rnode.go | 8 +++++ 8 files changed, 74 insertions(+), 66 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index fd423120d1..43250e0b13 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -38,6 +38,9 @@ type ByteReadWriter struct { // the Resources, otherwise they will be cleared. KeepReaderAnnotations bool + // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource + AddSeqIndentAnnotation bool + // Style is a style that is set on the Resource Node Document. Style yaml.Style @@ -48,16 +51,13 @@ 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, - AddSeqIndentAnnotation: rw.RetainSeqIndent, + AddSeqIndentAnnotation: rw.AddSeqIndentAnnotation, } val, err := b.Read() if rw.FunctionConfig == nil { @@ -117,6 +117,9 @@ type ByteReader struct { // annotation on Resources as they are Read. OmitReaderAnnotations bool + // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource + AddSeqIndentAnnotation 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 @@ -135,9 +138,6 @@ 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{} @@ -264,46 +264,35 @@ func (r *ByteReader) Read() ([]*yaml.RNode, error) { // 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 { + // step 1: derive the sequence indentation of the node 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() + // marshal the node with 2 space sequence indentation and calculate the diff + out, err := yaml.MarshalWithIndent(node.Document(), yaml.DefaultMapIndent, yaml.WideSeqIndent) if err != nil { return err } - twoSpaceIndentDiff := copyutil.PrettyFileDiff(out, value) + twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(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() + // marshal the node with 0 space sequence indentation and calculate the diff + out, err = yaml.MarshalWithIndent(node.Document(), yaml.DefaultMapIndent, yaml.CompactSeqIndent) if err != nil { return err } - noIndentDiff := copyutil.PrettyFileDiff(out, value) + noIndentDiff := copyutil.PrettyFileDiff(string(out), value) var style string if len(noIndentDiff) <= len(twoSpaceIndentDiff) { - style = yaml.CompactSequenceStyle + style = string(yaml.CompactSeqIndent) } else { - style = yaml.WideSequenceStyle + style = string(yaml.WideSeqIndent) } + // step 2: add the annotation to the node return node.PipeE( yaml.LookupCreate(yaml.MappingNode, yaml.MetadataField, yaml.AnnotationsField), yaml.SetField(kioutil.SeqIndentAnnotation, yaml.NewScalarRNode(style))) diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index 3d9bbdcc83..7b74e009a9 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/kyaml/kio/byteio_reader_test.go @@ -806,7 +806,7 @@ items: } } -func TestByteReader_RetainSeqIndent(t *testing.T) { +func TestByteReader_AddSeqIndent(t *testing.T) { type testCase struct { name string err string diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index fe7490fa3d..bb1759c3c6 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -581,7 +581,7 @@ spec: w := tc.instance w.Writer = &out w.Reader = &in - w.RetainSeqIndent = true + w.AddSeqIndentAnnotation = true nodes, err := w.Read() if !assert.NoError(t, err) { diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index 3d83039358..4f87f794d5 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -89,6 +89,9 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { if jsonEncodeSingleBareNode { encoder := json.NewEncoder(w.Writer) encoder.SetIndent("", " ") + if err := nodes[0].DeleteAnnotation(kioutil.SeqIndentAnnotation); err != nil { + return errors.Wrap(err) + } return errors.Wrap(encoder.Encode(nodes[0])) } @@ -97,8 +100,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 { + if err := unwrapAndEncodeYAML(nodes[i], encoder); err != nil { return err } } @@ -135,23 +137,21 @@ 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) { +// unwrapAndEncodeYAML encodes the yaml RNode in unwrapped format, +// as a pre-step, it clears the sets the sequence indentation for encoder, +// based on the kioutil.SeqIndentAnnotation and clears it before encoding. +func unwrapAndEncodeYAML(node *yaml.RNode, encoder *yaml.Encoder) error { anno := node.GetAnnotations() seqIndent := anno[kioutil.SeqIndentAnnotation] - if seqIndent == yaml.WideSequenceStyle { + if seqIndent == string(yaml.WideSeqIndent) { encoder.DefaultSeqIndent() } else { encoder.CompactSeqIndent() } - if err := node.PipeE(yaml.ClearAnnotation(kioutil.SeqIndentAnnotation)); err != nil { - return - } - if err := yaml.ClearEmptyAnnotations(node); err != nil { - return + if err := node.DeleteAnnotation(kioutil.SeqIndentAnnotation); err != nil { + return errors.Wrap(err) } + return encoder.Encode(node.Document()) } func copyRNodes(in []*yaml.RNode) []*yaml.RNode { diff --git a/kyaml/kio/kioutil/kioutil.go b/kyaml/kio/kioutil/kioutil.go index 8d599448c6..993cdfd848 100644 --- a/kyaml/kio/kioutil/kioutil.go +++ b/kyaml/kio/kioutil/kioutil.go @@ -23,7 +23,7 @@ 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 records the sequence nodes indentation of the input resource SeqIndentAnnotation AnnotationKey = "internal.config.kubernetes.io/seqindent" ) diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index 49b8457df1..08d20f39be 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -178,7 +178,8 @@ type LocalPackageReader struct { // the file FileSkipFunc LocalPackageSkipFileFunc - RetainSeqIndent bool + // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource + AddSeqIndentAnnotation bool } var _ Reader = LocalPackageReader{} @@ -269,7 +270,7 @@ func (r *LocalPackageReader) readFile(path string, _ os.FileInfo) ([]*yaml.RNode Reader: f, OmitReaderAnnotations: r.OmitReaderAnnotations, SetAnnotations: r.SetAnnotations, - AddSeqIndentAnnotation: r.RetainSeqIndent, + AddSeqIndentAnnotation: r.AddSeqIndentAnnotation, } return rr.Read() } diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 8ce609fa98..41d5363211 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -10,24 +10,14 @@ import ( "sigs.k8s.io/kustomize/kyaml/internal/forked/github.com/go-yaml/yaml" ) -const CompactSequenceStyle = "compact" -const WideSequenceStyle = "wide" - -const DefaultIndent = 2 -const DefaultSequenceStyle = CompactSequenceStyle +const ( + WideSeqIndent SeqIndentType = "wide" + CompactSeqIndent SeqIndentType = "compact" + DefaultMapIndent = 2 +) -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 -} +// SeqIndentType holds the indentation style for sequence nodes +type SeqIndentType string // Expose the yaml.v3 functions so this package can be used as a replacement @@ -53,13 +43,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(DefaultMapIndent) + e.CompactSeqIndent() return e } +// MarshalWithIndent marshals the input interface with provided indents +func MarshalWithIndent(in interface{}, mapIndent int, seqIndent SeqIndentType) ([]byte, error) { + var buf bytes.Buffer + err := NewEncoderWithIndent(&buf, mapIndent, seqIndent).Encode(in) + if err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// NewEncoderWithIndent returns the encoder with configurable indents +func NewEncoderWithIndent(w io.Writer, mapIndent int, seqIndent SeqIndentType) *yaml.Encoder { + encoder := NewEncoder(w) + encoder.SetIndent(mapIndent) + if seqIndent == WideSeqIndent { + 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 diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index 9d9cbcaf94..f3ba5b0797 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -505,6 +505,14 @@ func (rn *RNode) SetAnnotations(m map[string]string) error { return rn.setMapInMetadata(m, AnnotationsField) } +// DeleteAnnotation tries to delete the annotation and clears the empty metadata field. +func (rn *RNode) DeleteAnnotation(annotation string) error { + if err := rn.PipeE(ClearAnnotation(annotation)); err != nil { + return err + } + return ClearEmptyAnnotations(rn) +} + // GetLabels gets the metadata labels field. // If the field is missing, returns an empty map. // Use another method to check for missing metadata. From f81201b74dc925572ca7066ea7f8f5b8575822b9 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Thu, 8 Jul 2021 14:37:24 -0700 Subject: [PATCH 3/7] Add options, keep seqindent annotation equivalent to index annotation --- kyaml/kio/byteio_reader.go | 99 +++++++++++++++-------------- kyaml/kio/byteio_reader_test.go | 14 ++-- kyaml/kio/byteio_readwriter_test.go | 41 ------------ kyaml/kio/byteio_writer.go | 40 ++++++------ kyaml/kio/byteio_writer_test.go | 89 ++++++++++++++++++++++++++ kyaml/kio/pkgio_reader_test.go | 63 ++++++++++++++++++ kyaml/yaml/alias.go | 23 +++++-- kyaml/yaml/rnode.go | 8 --- 8 files changed, 248 insertions(+), 129 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 43250e0b13..620eea1bc9 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -199,17 +199,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 r.AddSeqIndentAnnotation { - if err := addSeqIndentAnno(values[i], node); err != nil { - return nil, errors.Wrap(err) - } - } - if err != nil { return nil, errors.Wrap(err) } @@ -260,45 +254,7 @@ 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 { - // step 1: derive the sequence indentation of the node - anno := node.GetAnnotations() - if anno[kioutil.SeqIndentAnnotation] != "" { - // the annotation already exists, so don't change it - return nil - } - - // marshal the node with 2 space sequence indentation and calculate the diff - out, err := yaml.MarshalWithIndent(node.Document(), yaml.DefaultMapIndent, yaml.WideSeqIndent) - if err != nil { - return err - } - twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(out), value) - - // marshal the node with 0 space sequence indentation and calculate the diff - out, err = yaml.MarshalWithIndent(node.Document(), yaml.DefaultMapIndent, yaml.CompactSeqIndent) - if err != nil { - return err - } - noIndentDiff := copyutil.PrettyFileDiff(string(out), value) - - var style string - if len(noIndentDiff) <= len(twoSpaceIndentDiff) { - style = string(yaml.CompactSeqIndent) - } else { - style = string(yaml.WideSeqIndent) - } - - // step 2: add the annotation to the node - 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) { +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 { @@ -321,6 +277,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.AddSeqIndentAnnotation { + // derive and add the seqindent annotation + seqIndentStyle := seqIndentAnno(node, originalYAML) + if seqIndentStyle != "" { + r.SetAnnotations[kioutil.SeqIndentAnnotation] = fmt.Sprintf("%s", seqIndentStyle) + } + } } var keys []string for k := range r.SetAnnotations { @@ -335,3 +299,46 @@ func (r *ByteReader) decode(index int, decoder *yaml.Decoder) (*yaml.RNode, erro } return yaml.NewRNode(node), nil } + +// seqIndentAnno derives the sequence indentation annotation value for the resource, +// originalYAML is the input yaml string and node is the decoded equivalent of originalYAML, +// the annotation value is decided by deriving the existing sequence indentation of resource +func seqIndentAnno(node *yaml.Node, originalYAML string) string { + rNode := &yaml.RNode{} + rNode.SetYNode(node) + // step 1: derive the sequence indentation of the node + anno := rNode.GetAnnotations() + if anno[kioutil.SeqIndentAnnotation] != "" { + // the annotation already exists, so don't change it + return "" + } + + // marshal the node with 2 space sequence indentation and calculate the diff + out, err := yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{ + MapIndent: yaml.DefaultMapIndent, + SeqIndent: yaml.WideSeqIndent, + }) + if err != nil { + return "" + } + twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) + + // marshal the node with 0 space sequence indentation and calculate the diff + out, err = yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{ + MapIndent: yaml.DefaultMapIndent, + SeqIndent: yaml.CompactSeqIndent, + }) + if err != nil { + return "" + } + noIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) + + var style string + if len(noIndentDiff) <= len(twoSpaceIndentDiff) { + style = string(yaml.CompactSeqIndent) + } else { + style = string(yaml.WideSeqIndent) + } + + return style +} diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index 7b74e009a9..e0ae8c0c2a 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/kyaml/kio/byteio_reader_test.go @@ -832,7 +832,8 @@ spec: - baz metadata: annotations: - internal.config.kubernetes.io/seqindent: wide + config.kubernetes.io/index: '0' + internal.config.kubernetes.io/seqindent: 'wide' `, }, { @@ -852,7 +853,8 @@ spec: - baz metadata: annotations: - internal.config.kubernetes.io/seqindent: compact + config.kubernetes.io/index: '0' + internal.config.kubernetes.io/seqindent: 'compact' `, }, { @@ -878,7 +880,8 @@ env: - bar metadata: annotations: - internal.config.kubernetes.io/seqindent: wide + config.kubernetes.io/index: '0' + internal.config.kubernetes.io/seqindent: 'wide' `, }, { @@ -904,7 +907,8 @@ env: - bar metadata: annotations: - internal.config.kubernetes.io/seqindent: compact + config.kubernetes.io/index: '0' + internal.config.kubernetes.io/seqindent: 'compact' `, }, } @@ -913,7 +917,7 @@ metadata: tc := testCases[i] t.Run(tc.name, func(t *testing.T) { rNodes, err := (&ByteReader{ - OmitReaderAnnotations: true, + OmitReaderAnnotations: false, AddSeqIndentAnnotation: true, Reader: bytes.NewBuffer([]byte(tc.input)), }).Read() diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index bb1759c3c6..e7657e1bcc 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -289,47 +289,6 @@ 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 { diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index 4f87f794d5..628ce438a1 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -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)) @@ -89,9 +100,6 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { if jsonEncodeSingleBareNode { encoder := json.NewEncoder(w.Writer) encoder.SetIndent("", " ") - if err := nodes[0].DeleteAnnotation(kioutil.SeqIndentAnnotation); err != nil { - return errors.Wrap(err) - } return errors.Wrap(encoder.Encode(nodes[0])) } @@ -100,8 +108,13 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { // don't wrap the elements if w.WrappingKind == "" { for i := range nodes { - if err := unwrapAndEncodeYAML(nodes[i], encoder); err != nil { - return err + if seqIndentsForNodes[i] == string(yaml.WideSeqIndent) { + encoder.DefaultSeqIndent() + } else { + encoder.CompactSeqIndent() + } + if err := encoder.Encode(nodes[i].Document()); err != nil { + return errors.Wrap(err) } } return nil @@ -137,23 +150,6 @@ func (w ByteWriter) Write(inputNodes []*yaml.RNode) error { return encoder.Encode(doc) } -// unwrapAndEncodeYAML encodes the yaml RNode in unwrapped format, -// as a pre-step, it clears the sets the sequence indentation for encoder, -// based on the kioutil.SeqIndentAnnotation and clears it before encoding. -func unwrapAndEncodeYAML(node *yaml.RNode, encoder *yaml.Encoder) error { - anno := node.GetAnnotations() - seqIndent := anno[kioutil.SeqIndentAnnotation] - if seqIndent == string(yaml.WideSeqIndent) { - encoder.DefaultSeqIndent() - } else { - encoder.CompactSeqIndent() - } - if err := node.DeleteAnnotation(kioutil.SeqIndentAnnotation); err != nil { - return errors.Wrap(err) - } - return encoder.Encode(node.Document()) -} - func copyRNodes(in []*yaml.RNode) []*yaml.RNode { out := make([]*yaml.RNode, len(in)) for i := range in { diff --git a/kyaml/kio/byteio_writer_test.go b/kyaml/kio/byteio_writer_test.go index 76d93d265c..241a9c31d9 100644 --- a/kyaml/kio/byteio_writer_test.go +++ b/kyaml/kio/byteio_writer_test.go @@ -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 // diff --git a/kyaml/kio/pkgio_reader_test.go b/kyaml/kio/pkgio_reader_test.go index 1da5113bac..0ac6c434bc 100644 --- a/kyaml/kio/pkgio_reader_test.go +++ b/kyaml/kio/pkgio_reader_test.go @@ -337,6 +337,69 @@ g: } } +func TestLocalPackageReader_Read_addSeqIndentAnnotation(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, AddSeqIndentAnnotation: 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() diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 41d5363211..6fb4f219a2 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -19,6 +19,15 @@ const ( // SeqIndentType holds the indentation style for sequence nodes type SeqIndentType string +// EncoderOptions are options that can be used to configure the encoder +type EncoderOptions struct { + // MapIndent is the indentation for YAML Mapping nodes + MapIndent int + + // SeqIndent is the indentation style for YAML Sequence nodes + SeqIndent SeqIndentType +} + // Expose the yaml.v3 functions so this package can be used as a replacement type Decoder = yaml.Decoder @@ -48,21 +57,21 @@ var NewEncoder = func(w io.Writer) *yaml.Encoder { return e } -// MarshalWithIndent marshals the input interface with provided indents -func MarshalWithIndent(in interface{}, mapIndent int, seqIndent SeqIndentType) ([]byte, error) { +// MarshalWithOptions marshals the input interface with provided options +func MarshalWithOptions(in interface{}, opts *EncoderOptions) ([]byte, error) { var buf bytes.Buffer - err := NewEncoderWithIndent(&buf, mapIndent, seqIndent).Encode(in) + err := NewEncoderWithOptions(&buf, opts).Encode(in) if err != nil { return nil, err } return buf.Bytes(), nil } -// NewEncoderWithIndent returns the encoder with configurable indents -func NewEncoderWithIndent(w io.Writer, mapIndent int, seqIndent SeqIndentType) *yaml.Encoder { +// NewEncoderWithOptions returns the encoder with provided options +func NewEncoderWithOptions(w io.Writer, opts *EncoderOptions) *yaml.Encoder { encoder := NewEncoder(w) - encoder.SetIndent(mapIndent) - if seqIndent == WideSeqIndent { + encoder.SetIndent(opts.MapIndent) + if opts.SeqIndent == WideSeqIndent { encoder.DefaultSeqIndent() } else { encoder.CompactSeqIndent() diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index f3ba5b0797..9d9cbcaf94 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -505,14 +505,6 @@ func (rn *RNode) SetAnnotations(m map[string]string) error { return rn.setMapInMetadata(m, AnnotationsField) } -// DeleteAnnotation tries to delete the annotation and clears the empty metadata field. -func (rn *RNode) DeleteAnnotation(annotation string) error { - if err := rn.PipeE(ClearAnnotation(annotation)); err != nil { - return err - } - return ClearEmptyAnnotations(rn) -} - // GetLabels gets the metadata labels field. // If the field is missing, returns an empty map. // Use another method to check for missing metadata. From c07ffa5c1ef96345534f3e166b8bda8381650631 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Fri, 9 Jul 2021 00:17:09 -0700 Subject: [PATCH 4/7] Update comments, tests, not expose indent option --- kyaml/kio/byteio_reader.go | 12 +++--------- kyaml/kio/byteio_readwriter_test.go | 4 ++-- kyaml/kio/pkgio_reader.go | 18 +++++++++++------- kyaml/yaml/alias.go | 12 +++++------- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index 620eea1bc9..f179cedba7 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -114,7 +114,7 @@ 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 // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource @@ -314,20 +314,14 @@ func seqIndentAnno(node *yaml.Node, originalYAML string) string { } // marshal the node with 2 space sequence indentation and calculate the diff - out, err := yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{ - MapIndent: yaml.DefaultMapIndent, - SeqIndent: yaml.WideSeqIndent, - }) + out, err := yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{SeqIndent: yaml.WideSeqIndent}) if err != nil { return "" } twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) // marshal the node with 0 space sequence indentation and calculate the diff - out, err = yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{ - MapIndent: yaml.DefaultMapIndent, - SeqIndent: yaml.CompactSeqIndent, - }) + out, err = yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{SeqIndent: yaml.CompactSeqIndent}) if err != nil { return "" } diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index e7657e1bcc..20f8cf8fd8 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -425,7 +425,7 @@ spec: `, }, { - name: "round_trip with mixed indentations in same resource, least diff wins", + name: "round_trip with mixed indentations in same resource, wide wins", input: ` apiVersion: apps/v1 kind: Deployment @@ -450,7 +450,7 @@ env: `, }, { - name: "round_trip with mixed indentations in same resource, least diff wins", + name: "round_trip with mixed indentations in same resource, compact wins", input: ` apiVersion: apps/v1 kind: Deployment diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index 08d20f39be..3dd9b99c71 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -40,6 +40,9 @@ type LocalPackageReadWriter struct { KeepReaderAnnotations bool `yaml:"keepReaderAnnotations,omitempty"` + // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource + AddSeqIndentAnnotation bool + // PackagePath is the path to the package directory. PackagePath string `yaml:"path,omitempty"` @@ -79,13 +82,14 @@ type LocalPackageReadWriter struct { func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) { nodes, err := LocalPackageReader{ - PackagePath: r.PackagePath, - MatchFilesGlob: r.MatchFilesGlob, - IncludeSubpackages: r.IncludeSubpackages, - ErrorIfNonResources: r.ErrorIfNonResources, - SetAnnotations: r.SetAnnotations, - PackageFileName: r.PackageFileName, - FileSkipFunc: r.FileSkipFunc, + PackagePath: r.PackagePath, + MatchFilesGlob: r.MatchFilesGlob, + IncludeSubpackages: r.IncludeSubpackages, + ErrorIfNonResources: r.ErrorIfNonResources, + SetAnnotations: r.SetAnnotations, + PackageFileName: r.PackageFileName, + FileSkipFunc: r.FileSkipFunc, + AddSeqIndentAnnotation: r.AddSeqIndentAnnotation, }.Read() if err != nil { return nil, errors.Wrap(err) diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index 6fb4f219a2..b4a60ad112 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -13,17 +13,15 @@ import ( const ( WideSeqIndent SeqIndentType = "wide" CompactSeqIndent SeqIndentType = "compact" - DefaultMapIndent = 2 + DefaultIndent = 2 ) // SeqIndentType holds the indentation style for sequence nodes type SeqIndentType string -// EncoderOptions are options that can be used to configure the encoder +// EncoderOptions are options that can be used to configure the encoder, +// do not expose new options without considerable justification type EncoderOptions struct { - // MapIndent is the indentation for YAML Mapping nodes - MapIndent int - // SeqIndent is the indentation style for YAML Sequence nodes SeqIndent SeqIndentType } @@ -52,7 +50,7 @@ var Unmarshal = yaml.Unmarshal var NewDecoder = yaml.NewDecoder var NewEncoder = func(w io.Writer) *yaml.Encoder { e := yaml.NewEncoder(w) - e.SetIndent(DefaultMapIndent) + e.SetIndent(DefaultIndent) e.CompactSeqIndent() return e } @@ -70,7 +68,7 @@ func MarshalWithOptions(in interface{}, opts *EncoderOptions) ([]byte, error) { // NewEncoderWithOptions returns the encoder with provided options func NewEncoderWithOptions(w io.Writer, opts *EncoderOptions) *yaml.Encoder { encoder := NewEncoder(w) - encoder.SetIndent(opts.MapIndent) + encoder.SetIndent(DefaultIndent) if opts.SeqIndent == WideSeqIndent { encoder.DefaultSeqIndent() } else { From 89b12cfc62aa128e5cfa590b513b78029a9344d2 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Fri, 9 Jul 2021 01:32:48 -0700 Subject: [PATCH 5/7] Change annotation name, error if conflicting options --- kyaml/kio/byteio_reader.go | 20 ++++++----- kyaml/kio/byteio_reader_test.go | 51 ++++++++++++++++++++++++----- kyaml/kio/byteio_readwriter_test.go | 2 +- kyaml/kio/pkgio_reader.go | 34 +++++++++---------- kyaml/kio/pkgio_reader_test.go | 4 +-- 5 files changed, 75 insertions(+), 36 deletions(-) diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index f179cedba7..a9cc0471b0 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -38,8 +38,8 @@ type ByteReadWriter struct { // the Resources, otherwise they will be cleared. KeepReaderAnnotations bool - // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource - AddSeqIndentAnnotation 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 @@ -55,9 +55,9 @@ type ByteReadWriter struct { func (rw *ByteReadWriter) Read() ([]*yaml.RNode, error) { b := &ByteReader{ - Reader: rw.Reader, - OmitReaderAnnotations: rw.OmitReaderAnnotations, - AddSeqIndentAnnotation: rw.AddSeqIndentAnnotation, + Reader: rw.Reader, + OmitReaderAnnotations: rw.OmitReaderAnnotations, + PreserveSeqIndent: rw.PreserveSeqIndent, } val, err := b.Read() if rw.FunctionConfig == nil { @@ -117,8 +117,8 @@ type ByteReader struct { // and internal.config.kubernetes.io/seqindent annotations on Resources as they are Read. OmitReaderAnnotations bool - // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource - AddSeqIndentAnnotation 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 @@ -174,6 +174,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 @@ -278,7 +282,7 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode if !r.OmitReaderAnnotations { r.SetAnnotations[kioutil.IndexAnnotation] = fmt.Sprintf("%d", index) - if r.AddSeqIndentAnnotation { + if r.PreserveSeqIndent { // derive and add the seqindent annotation seqIndentStyle := seqIndentAnno(node, originalYAML) if seqIndentStyle != "" { diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index e0ae8c0c2a..396fa58804 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/kyaml/kio/byteio_reader_test.go @@ -806,12 +806,17 @@ items: } } -func TestByteReader_AddSeqIndent(t *testing.T) { +// TestByteReader_AddSeqIndentAnnotation tests if the internal.config.kubernetes.io/seqindent +// annotation is added to resources appropriately, the expectedOutput indentation may not +// match with the annotation as it is not using byteio_writer, this test will only verify +// byteio_reader behavior to add annotation +func TestByteReader_AddSeqIndentAnnotation(t *testing.T) { type testCase struct { - name string - err string - input string - expectedOutput string + name string + err string + input string + expectedOutput string + OmitReaderAnnotations bool } testCases := []testCase{ @@ -911,16 +916,46 @@ metadata: internal.config.kubernetes.io/seqindent: 'compact' `, }, + { + name: "error if conflicting options", + 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 +`, + 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: false, - AddSeqIndentAnnotation: true, - Reader: bytes.NewBuffer([]byte(tc.input)), + 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, err := rNodes[0].String() assert.NoError(t, err) diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 20f8cf8fd8..040b8d81d6 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -540,7 +540,7 @@ spec: w := tc.instance w.Writer = &out w.Reader = &in - w.AddSeqIndentAnnotation = true + w.PreserveSeqIndent = true nodes, err := w.Read() if !assert.NoError(t, err) { diff --git a/kyaml/kio/pkgio_reader.go b/kyaml/kio/pkgio_reader.go index 3dd9b99c71..f2ed9f0047 100644 --- a/kyaml/kio/pkgio_reader.go +++ b/kyaml/kio/pkgio_reader.go @@ -40,8 +40,8 @@ type LocalPackageReadWriter struct { KeepReaderAnnotations bool `yaml:"keepReaderAnnotations,omitempty"` - // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource - AddSeqIndentAnnotation bool + // PreserveSeqIndent if true adds kioutil.SeqIndentAnnotation to each resource + PreserveSeqIndent bool // PackagePath is the path to the package directory. PackagePath string `yaml:"path,omitempty"` @@ -82,14 +82,14 @@ type LocalPackageReadWriter struct { func (r *LocalPackageReadWriter) Read() ([]*yaml.RNode, error) { nodes, err := LocalPackageReader{ - PackagePath: r.PackagePath, - MatchFilesGlob: r.MatchFilesGlob, - IncludeSubpackages: r.IncludeSubpackages, - ErrorIfNonResources: r.ErrorIfNonResources, - SetAnnotations: r.SetAnnotations, - PackageFileName: r.PackageFileName, - FileSkipFunc: r.FileSkipFunc, - AddSeqIndentAnnotation: r.AddSeqIndentAnnotation, + PackagePath: r.PackagePath, + MatchFilesGlob: r.MatchFilesGlob, + IncludeSubpackages: r.IncludeSubpackages, + ErrorIfNonResources: r.ErrorIfNonResources, + SetAnnotations: r.SetAnnotations, + PackageFileName: r.PackageFileName, + FileSkipFunc: r.FileSkipFunc, + PreserveSeqIndent: r.PreserveSeqIndent, }.Read() if err != nil { return nil, errors.Wrap(err) @@ -182,8 +182,8 @@ type LocalPackageReader struct { // the file FileSkipFunc LocalPackageSkipFileFunc - // AddSeqIndentAnnotation if true adds kioutil.SeqIndentAnnotation to each resource - AddSeqIndentAnnotation bool + // PreserveSeqIndent if true adds kioutil.SeqIndentAnnotation to each resource + PreserveSeqIndent bool } var _ Reader = LocalPackageReader{} @@ -270,11 +270,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, - AddSeqIndentAnnotation: r.AddSeqIndentAnnotation, + DisableUnwrapping: true, + Reader: f, + OmitReaderAnnotations: r.OmitReaderAnnotations, + SetAnnotations: r.SetAnnotations, + PreserveSeqIndent: r.PreserveSeqIndent, } return rr.Read() } diff --git a/kyaml/kio/pkgio_reader_test.go b/kyaml/kio/pkgio_reader_test.go index 0ac6c434bc..e5462d88af 100644 --- a/kyaml/kio/pkgio_reader_test.go +++ b/kyaml/kio/pkgio_reader_test.go @@ -337,7 +337,7 @@ g: } } -func TestLocalPackageReader_Read_addSeqIndentAnnotation(t *testing.T) { +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) @@ -351,7 +351,7 @@ func TestLocalPackageReader_Read_addSeqIndentAnnotation(t *testing.T) { } for _, p := range paths { // empty path - rfr := LocalPackageReader{PackagePath: p.path, AddSeqIndentAnnotation: true} + rfr := LocalPackageReader{PackagePath: p.path, PreserveSeqIndent: true} nodes, err := rfr.Read() if !assert.NoError(t, err) { return From 74e867833af4eac49a9dac7790cc9e09191a967c Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Fri, 9 Jul 2021 14:10:30 -0700 Subject: [PATCH 6/7] First sequence indent wins --- kyaml/kio/byteio_reader.go | 40 +------- kyaml/kio/byteio_readwriter_test.go | 25 +++++ kyaml/yaml/util.go | 48 ++++++++++ kyaml/yaml/util_test.go | 143 ++++++++++++++++++++++++++++ 4 files changed, 217 insertions(+), 39 deletions(-) create mode 100644 kyaml/yaml/util.go create mode 100644 kyaml/yaml/util_test.go diff --git a/kyaml/kio/byteio_reader.go b/kyaml/kio/byteio_reader.go index a9cc0471b0..7513f0d644 100644 --- a/kyaml/kio/byteio_reader.go +++ b/kyaml/kio/byteio_reader.go @@ -11,7 +11,6 @@ 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" @@ -284,7 +283,7 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode if r.PreserveSeqIndent { // derive and add the seqindent annotation - seqIndentStyle := seqIndentAnno(node, originalYAML) + seqIndentStyle := yaml.DeriveSeqIndentStyle(originalYAML) if seqIndentStyle != "" { r.SetAnnotations[kioutil.SeqIndentAnnotation] = fmt.Sprintf("%s", seqIndentStyle) } @@ -303,40 +302,3 @@ func (r *ByteReader) decode(originalYAML string, index int, decoder *yaml.Decode } return yaml.NewRNode(node), nil } - -// seqIndentAnno derives the sequence indentation annotation value for the resource, -// originalYAML is the input yaml string and node is the decoded equivalent of originalYAML, -// the annotation value is decided by deriving the existing sequence indentation of resource -func seqIndentAnno(node *yaml.Node, originalYAML string) string { - rNode := &yaml.RNode{} - rNode.SetYNode(node) - // step 1: derive the sequence indentation of the node - anno := rNode.GetAnnotations() - if anno[kioutil.SeqIndentAnnotation] != "" { - // the annotation already exists, so don't change it - return "" - } - - // marshal the node with 2 space sequence indentation and calculate the diff - out, err := yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{SeqIndent: yaml.WideSeqIndent}) - if err != nil { - return "" - } - twoSpaceIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) - - // marshal the node with 0 space sequence indentation and calculate the diff - out, err = yaml.MarshalWithOptions(rNode.Document(), &yaml.EncoderOptions{SeqIndent: yaml.CompactSeqIndent}) - if err != nil { - return "" - } - noIndentDiff := copyutil.PrettyFileDiff(string(out), originalYAML) - - var style string - if len(noIndentDiff) <= len(twoSpaceIndentDiff) { - style = string(yaml.CompactSeqIndent) - } else { - style = string(yaml.WideSeqIndent) - } - - return style -} diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 040b8d81d6..4488cce6e1 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -528,6 +528,31 @@ 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 `, }, } diff --git a/kyaml/yaml/util.go b/kyaml/yaml/util.go new file mode 100644 index 0000000000..df6c79d33d --- /dev/null +++ b/kyaml/yaml/util.go @@ -0,0 +1,48 @@ +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.HasSuffix(trimmedKeyLine, "|") || strings.HasSuffix(trimmedKeyLine, "|-") { + // this is not a sequence node, it is a wrapped sequence node string + // ignore it + continue + } + + if numSpacesBeforeSeqElem == numSpacesBeforeKeyElem { + return string(CompactSeqIndent) + } + + if numSpacesBeforeSeqElem-numSpacesBeforeKeyElem == 2 { + return string(WideSeqIndent) + } + } + + return string(CompactSeqIndent) +} diff --git a/kyaml/yaml/util_test.go b/kyaml/yaml/util_test.go new file mode 100644 index 0000000000..a83725c287 --- /dev/null +++ b/kyaml/yaml/util_test.go @@ -0,0 +1,143 @@ +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", + input: `apiVersion: apps/v1 +kind: Deployment +spec: |- + - foo + - bar +env: +- foo +- bar +- baz +`, + 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)) + }) + } +} From 29be7fabe418b350df912ce16f53ab21c9a77c54 Mon Sep 17 00:00:00 2001 From: Phani Teja Marupaka Date: Mon, 12 Jul 2021 23:27:09 -0700 Subject: [PATCH 7/7] Suggested changes --- kyaml/kio/byteio_reader_test.go | 76 ++++------------------------- kyaml/kio/byteio_readwriter_test.go | 35 +------------ kyaml/kio/byteio_writer.go | 2 +- kyaml/yaml/alias.go | 12 ++--- kyaml/yaml/util.go | 9 ++-- kyaml/yaml/util_test.go | 36 ++++++++++++-- 6 files changed, 54 insertions(+), 116 deletions(-) diff --git a/kyaml/kio/byteio_reader_test.go b/kyaml/kio/byteio_reader_test.go index 396fa58804..f509f25cb8 100644 --- a/kyaml/kio/byteio_reader_test.go +++ b/kyaml/kio/byteio_reader_test.go @@ -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) { @@ -807,15 +808,13 @@ items: } // TestByteReader_AddSeqIndentAnnotation tests if the internal.config.kubernetes.io/seqindent -// annotation is added to resources appropriately, the expectedOutput indentation may not -// match with the annotation as it is not using byteio_writer, this test will only verify -// byteio_reader behavior to add annotation +// annotation is added to resources appropriately func TestByteReader_AddSeqIndentAnnotation(t *testing.T) { type testCase struct { name string err string input string - expectedOutput string + expectedAnnoValue string OmitReaderAnnotations bool } @@ -829,17 +828,7 @@ spec: - bar - baz `, - expectedOutput: `apiVersion: apps/v1 -kind: Deployment -spec: -- foo -- bar -- baz -metadata: - annotations: - config.kubernetes.io/index: '0' - internal.config.kubernetes.io/seqindent: 'wide' -`, + expectedAnnoValue: "wide", }, { name: "read with compact indentation", @@ -850,17 +839,7 @@ spec: - bar - baz `, - expectedOutput: `apiVersion: apps/v1 -kind: Deployment -spec: -- foo -- bar -- baz -metadata: - annotations: - config.kubernetes.io/index: '0' - internal.config.kubernetes.io/seqindent: 'compact' -`, + expectedAnnoValue: "compact", }, { name: "read with mixed indentation, wide wins", @@ -874,20 +853,7 @@ env: - foo - bar `, - expectedOutput: `apiVersion: apps/v1 -kind: Deployment -spec: -- foo -- bar -- baz -env: -- foo -- bar -metadata: - annotations: - config.kubernetes.io/index: '0' - internal.config.kubernetes.io/seqindent: 'wide' -`, + expectedAnnoValue: "wide", }, { name: "read with mixed indentation, compact wins", @@ -901,20 +867,7 @@ env: - foo - bar `, - expectedOutput: `apiVersion: apps/v1 -kind: Deployment -spec: -- foo -- bar -- baz -env: -- foo -- bar -metadata: - annotations: - config.kubernetes.io/index: '0' - internal.config.kubernetes.io/seqindent: 'compact' -`, + expectedAnnoValue: "compact", }, { name: "error if conflicting options", @@ -924,16 +877,6 @@ spec: - foo - bar - baz -env: - - foo - - bar -`, - expectedOutput: `apiVersion: apps/v1 -kind: Deployment -spec: -- foo -- bar -- baz env: - foo - bar @@ -957,9 +900,8 @@ env: return } assert.NoError(t, err) - actual, err := rNodes[0].String() - assert.NoError(t, err) - assert.Equal(t, tc.expectedOutput, actual) + actual := rNodes[0].GetAnnotations()[kioutil.SeqIndentAnnotation] + assert.Equal(t, tc.expectedAnnoValue, actual) }) } } diff --git a/kyaml/kio/byteio_readwriter_test.go b/kyaml/kio/byteio_readwriter_test.go index 4488cce6e1..e096f2707f 100644 --- a/kyaml/kio/byteio_readwriter_test.go +++ b/kyaml/kio/byteio_readwriter_test.go @@ -425,14 +425,12 @@ spec: `, }, { - name: "round_trip with mixed indentations in same resource, wide wins", + name: "round_trip with mixed indentations in same resource, wide wins as it is first", input: ` apiVersion: apps/v1 kind: Deployment spec: - foo - - bar - - baz env: - foo - bar @@ -442,22 +440,18 @@ apiVersion: apps/v1 kind: Deployment spec: - foo - - bar - - baz env: - foo - bar `, }, { - name: "round_trip with mixed indentations in same resource, compact wins", + name: "round_trip with mixed indentations in same resource, compact wins as it is first", input: ` apiVersion: apps/v1 kind: Deployment spec: - foo -- bar -- baz env: - foo - bar @@ -467,31 +461,6 @@ 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 diff --git a/kyaml/kio/byteio_writer.go b/kyaml/kio/byteio_writer.go index 628ce438a1..33e5a8999d 100644 --- a/kyaml/kio/byteio_writer.go +++ b/kyaml/kio/byteio_writer.go @@ -108,7 +108,7 @@ 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.WideSeqIndent) { + if seqIndentsForNodes[i] == string(yaml.WideSequenceStyle) { encoder.DefaultSeqIndent() } else { encoder.CompactSeqIndent() diff --git a/kyaml/yaml/alias.go b/kyaml/yaml/alias.go index b4a60ad112..5a37301187 100644 --- a/kyaml/yaml/alias.go +++ b/kyaml/yaml/alias.go @@ -11,19 +11,19 @@ import ( ) const ( - WideSeqIndent SeqIndentType = "wide" - CompactSeqIndent SeqIndentType = "compact" - DefaultIndent = 2 + WideSequenceStyle SequenceIndentStyle = "wide" + CompactSequenceStyle SequenceIndentStyle = "compact" + DefaultIndent = 2 ) // SeqIndentType holds the indentation style for sequence nodes -type SeqIndentType string +type SequenceIndentStyle string // 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 SeqIndentType + SeqIndent SequenceIndentStyle } // Expose the yaml.v3 functions so this package can be used as a replacement @@ -69,7 +69,7 @@ func MarshalWithOptions(in interface{}, opts *EncoderOptions) ([]byte, error) { func NewEncoderWithOptions(w io.Writer, opts *EncoderOptions) *yaml.Encoder { encoder := NewEncoder(w) encoder.SetIndent(DefaultIndent) - if opts.SeqIndent == WideSeqIndent { + if opts.SeqIndent == WideSequenceStyle { encoder.DefaultSeqIndent() } else { encoder.CompactSeqIndent() diff --git a/kyaml/yaml/util.go b/kyaml/yaml/util.go index df6c79d33d..bada8711c8 100644 --- a/kyaml/yaml/util.go +++ b/kyaml/yaml/util.go @@ -29,20 +29,21 @@ func DeriveSeqIndentStyle(originalYAML string) string { numSpacesBeforeKeyElem := len(keyLine) - len(strings.TrimLeft(keyLine, " ")) trimmedKeyLine := strings.Trim(keyLine, " ") - if strings.HasSuffix(trimmedKeyLine, "|") || strings.HasSuffix(trimmedKeyLine, "|-") { + 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(CompactSeqIndent) + return string(CompactSequenceStyle) } if numSpacesBeforeSeqElem-numSpacesBeforeKeyElem == 2 { - return string(WideSeqIndent) + return string(WideSequenceStyle) } } - return string(CompactSeqIndent) + return string(CompactSequenceStyle) } diff --git a/kyaml/yaml/util_test.go b/kyaml/yaml/util_test.go index a83725c287..ef0d1fb5d2 100644 --- a/kyaml/yaml/util_test.go +++ b/kyaml/yaml/util_test.go @@ -79,16 +79,42 @@ env: expectedOutput: `compact`, }, { - name: "skip wrapped sequence strings", + name: "skip wrapped sequence strings, pipe hyphen", input: `apiVersion: apps/v1 kind: Deployment spec: |- - foo - bar -env: -- foo -- bar -- baz +`, + 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`, },